fix(auth): avoid nil deref when OIDC UserInfo fetch fails

[cursor]

Bug and impact

When an OIDC provider does not return an id_token and the UserInfo endpoint call fails (network error, 401, misconfiguration), oidcRedirect accessed userInfo.Profile before checking the error. That dereferenced a nil *oidc.UserInfo and panicked the Semaphore server process during the login redirect.

Root cause

In the no-id_token branch of oidcRedirect, username/name defaults were applied unconditionally after the UserInfo() call, even when err != nil left userInfo nil. This path predates the external-identity work but was left unfixed when the branch was extended in #4025.

Fix

• Extract oidcClaimsFromUserInfo so UserInfo errors return immediately without touching userInfo.
• Add regression tests for the error path and the happy path.

Validation

• go test ./api -run TestOidcClaimsFromUserInfo
• go test ./db/sql -run TestMigration_2_20

  

---

Note

Low Risk
Small, localized auth-path refactor with regression tests; behavior on success is unchanged and failures now return 502 instead of panicking.

Overview
Fixes a server panic on OIDC login when there is no id_token and the UserInfo request fails: oidcRedirect used to read userInfo.Profile even when UserInfo() returned an error and userInfo was nil.

UserInfo handling is moved into oidcClaimsFromUserInfo, which returns the fetch error immediately and only then builds claims (empty email → claimOidcUserInfo, otherwise direct fields plus random username / default name). oidcRedirect calls that helper instead of inlined logic.

Adds TestOidcClaimsFromUserInfo_UserInfoFailure and TestOidcClaimsFromUserInfo_WithEmail.

^{Reviewed by Cursor Bugbot for commit 4e7499a. Configure here.}

[cursor]

Security review

Outcome: No medium, high, or critical vulnerabilities found in the added/modified code.

Scope reviewed: api/login.go (oidcClaimsFromUserInfo extraction and oidcRedirect call site) and api/login_test.go.

Prior threads: No unresolved automation review threads were present on this PR.

Analysis

This change fixes a nil-pointer dereference in the OIDC callback path when _oidc.UserInfo fails (no id_token in the OAuth2 response). Previously, claim population was guarded by if err == nil, but userInfo.Profile was read unconditionally afterward, which could panic and deny service to the auth handler.

The refactor preserves the successful-path behavior:

• UserInfo errors are returned immediately and oidcRedirect responds with HTTP 502, so no session is created.
• Successful UserInfo responses still populate claims via claimOidcUserInfo or direct field mapping, with the same sub empty-check and resolveExternalUser gating before createSession.

No new attacker-controlled sinks, authn/authz bypasses, injection paths, or secret-exposure risks were introduced by this diff.

  

_{Sent by Cursor Automation: Find vulnerabilities}

[Copilot]

Pull request overview

Fixes a crash in the OIDC login redirect flow when an OIDC provider does not return an id_token and the subsequent UserInfo fetch fails, by ensuring UserInfo errors are handled before any dereference of the returned *oidc.UserInfo.

Changes:

• Extracts oidcClaimsFromUserInfo to centralize claim construction for the no-id_token path and return early on UserInfo errors.
• Refactors oidcRedirect to use the new helper instead of inlined logic.
• Adds regression tests covering the UserInfo error path and a successful UserInfo response with email.

Reviewed changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated 1 comment.

File	Description
api/login.go	Adds oidcClaimsFromUserInfo and uses it in oidcRedirect to prevent nil dereference on UserInfo failure.
api/login_test.go	Adds unit tests for the new helper’s error and success paths.