Skip to content

Commit

Permalink
Disallow using OmniAuth login as 2nd factor with password authentication
Browse files Browse the repository at this point in the history
When using the confirm password feature, password authentication can in
some cases be used as an additional factor, such as when using
passwordless login with WebAuthn.

An argument could be made that when logged in via OmniAuth, password
authentication could also be used as an additional factor, since you've
provided evidence that you "own" something (an account on 3rd party
service). However, for people that reuse passwords, allowing OmniAuth
login to be used as 2nd factor can be a security vulnerability, because
an attacker could use the same email & password to log into both the
main app and the external service, fulfilling both factors, even if the
user has a stronger multifactor authentication method setup.

Since developers will probably not be aware they're allowing this when
they have enabled both OmniAuth and confirm_password features, I think
it's safer to disable this behaviour. I don't know of use cases where
developers want to allow using OmniAuth login as 2nd factor, but if
this feature gets requested, we can easily add a configuration option to
enable it.
  • Loading branch information
janko committed Nov 28, 2022
1 parent 7bba512 commit ed6be6b
Show file tree
Hide file tree
Showing 2 changed files with 24 additions and 0 deletions.
5 changes: 5 additions & 0 deletions lib/rodauth/features/omniauth.rb
Original file line number Diff line number Diff line change
Expand Up @@ -112,6 +112,11 @@ def possible_authentication_methods

private

def before_confirm_password
authenticated_by.delete("omniauth")
super if defined?(super)
end

def allow_email_auth?
(defined?(super) ? super : true) && omniauth_account_identities_ds.empty?
end
Expand Down
19 changes: 19 additions & 0 deletions test/omniauth_test.rb
Original file line number Diff line number Diff line change
Expand Up @@ -217,4 +217,23 @@
omniauth_login "/auth/developer"
assert_equal 1, identity_id
end

it "disallows being used as 2nd factor with password authentication" do
rodauth do
enable :omniauth, :confirm_password
omniauth_provider :developer
end
roda do |r|
r.rodauth
rodauth.require_password_authentication
r.root { rodauth.authenticated_by.join(",") }
end

omniauth_login "/auth/developer"
assert_equal "/confirm-password", page.current_path

fill_in "Password", with: "secret"
click_on "Confirm Password"
assert_equal "password", page.html
end
end

0 comments on commit ed6be6b

Please sign in to comment.