Fix order of audit logging for OIDC re-authentication

Jason Knablrequested to merge
578322-fix-oidc-signin-log into master

What does this MR do and why?

Problem

When an admin user attempts to re-authenticate when entering admin mode, and chooses to re-auth using an external IdP, and the user authenticated at the IdP does not match the GitLab user, the Audit Log incorrectly records Signed in with OPENID_CONNECT, even though authentication fails.

Fix

For the admin mode authentication flow, don't log until the whole flow, including identity linking, has succeeded.

Question for reviewers

I'm wondering if the call to track_event() call should always happen after the identity linking logic in the OmniauthCallbacksController#oauth_flow method.

If the intention of the event tracking is to record an event after both a) successful IdP auth and b) a linked identity, then I think the Signed in with OPENID_CONNECT... log will be recorded for other non-admin mode scenarios even when an identity is not successfuly linked and overall auth fails.

Since the track_event() method wasn't placed before the identity linking logic, I'm hesitant to move it, since there might've been a reason it was placed where it is.

Since the original report of this issue focused on admin mode, I'm only addressing the admin mode concern in this MR. But maybe this is worth a follow-up.

References

Screenshots or screen recordings

How to set up and validate locally

You need an IdP with OIDC support to test the external auth for this flow. I'm using Keycloak, but instructions might be similar for other IdPs:

Set up keycloak as OIDC IdP

  1. Create a docker-compose.yml under gdk/ directory containing the following:
`docker-compose.yml` 
---
services:
  keycloak:
    image: keycloak/keycloak:25.0
    command: start-dev
    environment:
      KEYCLOAK_ADMIN: admin 
      KEYCLOAK_ADMIN_PASSWORD: admin
    ports:
      - 8080:8080
    volumes:
      - ./keycloak/data:/opt/keycloak/data
      - ```
  1. Run docker-compose up -d
  2. Visit Keycloak and sign in at http://localhost:8080
  3. Create new Realm called Gitlab
  4. Create new Client called gitlab-client. Use redirect URL: http://gdk.test:3443/users/auth/openid_connect/callback
  5. Create a user with an email and password (e.g.: bob-oidc@example.com)
  6. Set up gitlab.yml configuration to enable OIDC IdP:
`gitlab.yml` 
development:
  <<: *base
  omniauth:
    allow_single_sign_on: [ "openid_connect" ]
    auto_link_user: [ "openid_connect" ]
    block_auto_created_users: false
    providers:
    - { name: "openid_connect",
        label: "Keycloak",
        args: {
          name: "openid_connect",
          scope: [ "openid", "profile", "email" ],
          response_type: "code",
          issuer: "http://localhost:8080/realms/Gitlab",
          discovery: false,
          uid_field: "preferred_username",
          pkce: true,
          client_options: {
            host: "localhost",
            scheme: "http",
            port: "8080",
            identifier: "gitlab-client",
            secret: <INSERT_KEYCLOAK_CLIENT_SECRET_HERE>,
            redirect_uri: "https://gdk.test:3443/users/auth/openid_connect/callback",
            authorization_endpoint: "/realms/Gitlab/protocol/openid-connect/auth",
            token_endpoint: "/realms/Gitlab/protocol/openid-connect/token",
            userinfo_endpoint: "/realms/Gitlab/protocol/openid-connect/userinfo",
            jwks_uri: "http://localhost:8080/realms/Gitlab/protocol/openid-connect/certs",
            end_session_endpoint: "/realms/Gitlab/protocol/openid-connect/logout"
          }
        }
    }

Create corresponding user in Gitlab

  1. Visit GDK and log in normally with username+password as root (or another admin user).
  2. Create a corresponding user for your external IdP user, bob-oidc@example.com.
  3. Log out.
  4. Log in as bob-oidc@example.com using the OIDC flow by clicking the Keycloack button on the sign-in screen. This creates a session in the IdP.
  5. Log out.

Reproduce the bug (when IdP session identity doesn't match GitLab user identity)

Note: try this flow on master first to see the bug behaviour.

  1. Make sure admin mode is enabled for your instance by:
  2. gdk rails c
  3. >::Gitlab::CurrentSettings.update!(admin_mode: true)
  4. gdk restart
  5. Go back to the browser and log in as root using username+password
  6. Click Edit Profile --> Enter Admin Mode
  7. Click Keycloak to attempt logging in using OIDC. There is probably an active Keycloak session for bob-oidc@example.com but if there isn't, try entering bob-oidc@example.com's credentials at the Keycloak login screen.
  8. You'll be redirected back to GitLab re-auth screen and you will see an authentication error.
  9. Authenticate for real using root's password
  10. Visit Admin --> Monitoring --> Audit Log
  11. Observe the Signed in with OPENID_CONNECT authentication log event, even though you did not successfully sign in with OIDC.

Validate the fix (when IdP session identity doesn't match GitLab user identity)

  1. Check out this branch.
  2. Follow steps 4 through 10 again, but now notice there is no Signed in with OPENID_CONNECT authentication log event. This is correct, because your OIDC authentication was not successful when you re-authed for admin mode.

Validate the fix doesn't break existing flow (when IdP session identity does match GitLab user identity)

  1. Clear any active sessions from your IdP.
  2. Create a new admin user in GitLab. I used adam-admin@example.com.
  3. Create a new identity in the IdP for the new GitLab User.
  4. Back in GitLab, link the IdP identity to your GitLab user via Edit Profile --> Account --> Service sign-in --> Connected accounts
  5. Observe your IdP account is linked to the GitLab admin account and SSO works.
  6. In GitLab, visit Edit Profile --> Enter Admin Mode.
  7. Re-authenticate using your OIDC IdP.
  8. Observe the authentication was successful.
  9. Visit Admin --> Monitoring --> Audit Log
  10. Observe the Signed in with OPENID_CONNECT authentication log event exists from your admin mode re-auth. This confirms the OIDC event is still being tracked for cases where the external IdP identity matches the GitLab user identity.

MR acceptance checklist

Evaluate this MR against the MR acceptance checklist. It helps you analyze changes to reduce risks in quality, performance, reliability, security, and maintainability.

Related to #578322 (closed)

Edited by Jason Knabl

Merge request reports