FIX: Ensure auth completes correctly when going via /user-api-key/new for new users (#31759)

A new user joining a community via DiscourseHub and logging in via oauth
goes through this process. This would break down for two reasons.

Reason 1: in some cases, especially on Safari mobile, the redirect in
the omniauth callback was happening too early. A new user may not be
signed in yet by that point, which means the redirect to
`/user-api-key/new` triggers a redirect to `/login` which ends up in a
bit of an infinite loop. Not all browsers exhibited this behaviour, but
Safari definitely did.

Reason 2: `/user-api-key/new` is gated via group membership using the
`user_api_key_allowed_groups` site setting. By default that is set to
include `trust_level_0`, however, auto group assignment wasn't taking
place for all user `create` events (only some that go through staged
users).
This commit is contained in:
Penar Musaraj
2025-03-12 11:58:59 -04:00
committed by GitHub
parent 9bb4257810
commit 38ba191be0
3 changed files with 70 additions and 1 deletions

View File

@ -86,7 +86,12 @@ class Users::OmniauthCallbacksController < ApplicationController
cookies["_bypass_cache"] = true
cookies[:authentication_data] = { value: client_hash.to_json, path: Discourse.base_path("/") }
redirect_to @origin
if !current_user && @origin.start_with?("/user-api-key/new")
redirect_to path("/")
else
redirect_to @origin
end
end
def valid_origin?(uri)

View File

@ -123,6 +123,8 @@ class Auth::Result
user.update(associated_group_ids: associated_group_ids)
AssociatedGroup.where(id: associated_group_ids).update_all("last_used = CURRENT_TIMESTAMP")
end
Group.user_trust_level_change!(user.id, user.trust_level) if user && !user.staged
end
def can_edit_name

View File

@ -177,6 +177,68 @@ shared_examples "login scenarios" do |login_page_object|
expect(page).to have_css("#topic-title")
end
context "with user api key and omniauth" do
include OmniauthHelpers
let :public_key do
<<~TXT
-----BEGIN PUBLIC KEY-----
MIGfMA0GCSqGSIb3DQEBAQUAA4GNADCBiQKBgQDh7BS7Ey8hfbNhlNAW/47pqT7w
IhBz3UyBYzin8JurEQ2pY9jWWlY8CH147KyIZf1fpcsi7ZNxGHeDhVsbtUKZxnFV
p16Op3CHLJnnJKKBMNdXMy0yDfCAHZtqxeBOTcCo1Vt/bHpIgiK5kmaekyXIaD0n
w0z/BYpOgZ8QwnI5ZwIDAQAB
-----END PUBLIC KEY-----
TXT
end
let :private_key do
<<~TXT
-----BEGIN RSA PRIVATE KEY-----
MIICWwIBAAKBgQDh7BS7Ey8hfbNhlNAW/47pqT7wIhBz3UyBYzin8JurEQ2pY9jW
WlY8CH147KyIZf1fpcsi7ZNxGHeDhVsbtUKZxnFVp16Op3CHLJnnJKKBMNdXMy0y
DfCAHZtqxeBOTcCo1Vt/bHpIgiK5kmaekyXIaD0nw0z/BYpOgZ8QwnI5ZwIDAQAB
AoGAeHesbjzCivc+KbBybXEEQbBPsThY0Y+VdgD0ewif2U4UnNhzDYnKJeTZExwQ
vAK2YsRDV3KbhljnkagQduvmgJyCKuV/CxZvbJddwyIs3+U2D4XysQp3e1YZ7ROr
YlOIoekHCx1CNm6A4iImqGxB0aJ7Owdk3+QSIaMtGQWaPTECQQDz2UjJ+bomguNs
zdcv3ZP7W3U5RG+TpInSHiJXpt2JdNGfHItozGJCxfzDhuKHK5Cb23bgldkvB9Xc
p/tngTtNAkEA7S4cqUezA82xS7aYPehpRkKEmqzMwR3e9WeL7nZ2cdjZAHgXe49l
3mBhidEyRmtPqbXo1Xix8LDuqik0IdnlgwJAQeYTnLnHS8cNjQbnw4C/ECu8Nzi+
aokJ0eXg5A0tS4ttZvGA31Z0q5Tz5SdbqqnkT6p0qub0JZiZfCNNdsBe9QJAaGT5
fJDwfGYW+YpfLDCV1bUFhMc2QHITZtSyxL0jmSynJwu02k/duKmXhP+tL02gfMRy
vTMorxZRllgYeCXeXQJAEGRXR8/26jwqPtKKJzC7i9BuOYEagqj0nLG2YYfffCMc
d3JGCf7DMaUlaUE8bJ08PtHRJFSGkNfDJLhLKSjpbw==
-----END RSA PRIVATE KEY-----
TXT
end
let :args do
{
scopes: "one_time_password",
client_id: "x" * 32,
auth_redirect: "discourse://auth_redirect",
application_name: "foo",
public_key: public_key,
nonce: SecureRandom.hex,
}
end
before do
OmniAuth.config.test_mode = true
SiteSetting.auth_skip_create_confirm = true
SiteSetting.enable_google_oauth2_logins = true
SiteSetting.enable_local_logins = false
end
after { reset_omniauth_config(:google_oauth2) }
it "completes signup and redirects to the user api key authorization form" do
mock_google_auth
visit("/user-api-key/new?#{args.to_query}")
expect(page).to have_css(".authorize-api-key .scopes")
end
end
end
context "with two-factor authentication" do