GhostManager / Ghostwriter

The SpecterOps project management and reporting engine
https://ghostwriter.wiki
BSD 3-Clause "New" or "Revised" License
1.3k stars 178 forks source link

Connecting To Exsiting Accounts not Working with Adapter #521

Open er4z0r opened 4 days ago

er4z0r commented 4 days ago

Describe the bug The changelog for 4.3.0-RC1 states:

If an existing account has the same email address, the accounts will be linked

I found this to not work in my case. Let's start with my setup.

SOCIALACCOUNT_PROVIDERS = { 
        'google': { 
            'SCOPE': ['profile','email'],
            'AUTH_PARAMS': { 'access_type': 'online' }
        }
}

SSO_PROVIDERS = ["allauth.socialaccount.providers.google"]
INSTALLED_APPS = INSTALLED_APPS + SSO_PROVIDERS
SITE_ID = 1

SOCIAL_AUTH_AUTHENTICATION_BACKENDS = "social_core.backends.google.GoogleOAuth2"

SOCIAL_ACCOUNT_DOMAIN_ALLOWLIST= ["mycorpdomain.de"]
ACCOUNT_EMAIL_REQUIRED = True
SOCIAL_ACCOUNT_ALLOW_REGISTRATION = True

Steps to reproduce the behavior:

  1. Create a user named testuser, with email testuser.tilman@mycorpdomain.de. Have no social logins assigned to that user.
  2. Log out of the system.
  3. Click on Google
  4. Log in with testuser.tilman@mycorpdomain.de
  5. Confirm the OAuth Consent Dialogue with Google
  6. Get redirected to the Sign Up screen with the Email field prefilled.
  7. Set username testuser and click "Sign Up"
  8. Receive error "An account already exists with this email. Please sign into that account frist, then connect your Google account"

Expected Behavior

In step 5-7 I'd actually expect there to be no "Sign Up" (see Additional Context) but a "Connect Account" form where you basically confirm, that you want to link your local account to your Google Account. I think all-auth does have a /connect endpoint. Actually it might also be nice to have that reachable from the user's profile.

Workaround I'm considering not pre-creating accounts locally and just allowing people to do "Sign Up" via Google for Work, but I'm not entirely comfortable with it.

Screenshots ghostwriter_sso_existing_user_mtd_google_allow_reg_redacted ghostwriter_sso_existing_user_mtd_google_allow_reg_error_redacted

Server Specs:

Additional context Originally I wanted to have the tightest control possible over who gets an account. So I'd preferably have registration turned of and only allow existing users to connect their Google for Work account. That way I can assure that only accounts I created will have access to the system. I'm pondering to drop that requirement now that RBAC assigns new users the low 'user' role by default.

chrismaddalena commented 4 days ago

If a user already exists with the testuser.tilman@mycorpdomain.de email address that user should be matched with the social login with the matching address. That's all handled by django-allauth, not the custom adapter. The custom adapter adds a check to see if more than one user has that email address and raises an error if it finds two or more accounts. Otherwise, allauth will connect the oldest account with the social login. I didn't like that.

I think there are two possibilities:

  1. Google's social login behaves differently and the email is empty or doesn't match, so allauth determines there is no pre-existing account
  2. The email address is treated as unverified and allauth does not accept it

The second is the most likely. Please try this config change. Does it work as expected?

SOCIALACCOUNT_PROVIDERS = { 
        'google': { 
            'SCOPE': ['profile','email'],
            'AUTH_PARAMS': { 'access_type': 'online' }
            'EMAIL_AUTHENTICATION': True,
            'VERIFIED_EMAIL': True,
        }
}

I'll see if I can try Google auth.

chrismaddalena commented 4 days ago

I used those settings to add Google to my SSO providers, made a new user with my Google email address, logged-in with my Google account, and the existing account connected.

The only difference is you have this config line:

SOCIAL_AUTH_AUTHENTICATION_BACKENDS = "social_core.backends.google.GoogleOAuth2"

I don't know if django-allauth uses that or if it matters. I don't think it has an effect on this. I tried adding it to my config and didn't notice any changes to the results or the login flow.

er4z0r commented 4 days ago

If a user already exists with the testuser.tilman@mycorpdomain.de email address that user should be matched with the social login with the matching address. That's all handled by django-allauth, not the custom adapter. The custom adapter adds a check to see if more than one user has that email address and raises an error if it finds two or more accounts. Otherwise, allauth will connect the oldest account with the social login. I didn't like that.

I think there are two possibilities:

1. Google's social login behaves differently and the email is empty or doesn't match, so `allauth` determines there is no pre-existing account

Please try this config change. Does it work as expected?

SOCIALACCOUNT_PROVIDERS = { 
        'google': { 
            'SCOPE': ['profile','email'],
            'AUTH_PARAMS': { 'access_type': 'online' }
            'EMAIL_AUTHENTICATION': True,
            'VERIFIED_EMAIL': True,
        }
}

I'll see if I can try Google auth. I had totally forgotten about that verification aspect! Thank you! It worked :)

SOCIAL_AUTH_AUTHENTICATION_BACKENDS = "social_core.backends.google.GoogleOAuth2"

Where did you find that? It can't even remember where I found the one I'm using. It does not seem to be listed in the all-auth documentation for Google.

chrismaddalena commented 4 days ago

I think your config was based on an older version. Prior versions of Ghostwriter had an older django-allauth that was a release just prior to come consolidation they did. I know azure got rolled into microsoft where microsoft now encompasses anything Azure or Microsoft Graph. Maybe these settings are newer than what you looked at from while back.

I have some details here: https://www.ghostwriter.wiki/features/access-authentication-and-session-controls/single-sign-on#sso-registration-and-domain-allowlist

It's discussed here in the allauth docs: https://docs.allauth.org/en/latest/socialaccount/configuration.html