fix: prevent oauth nil user panic

[Zibbp]

#1051

[coderabbitai]

Walkthrough

Refactors OAuth user authentication by extracting role-detection logic into a centralized roleFromGroups helper for both creation and update paths. Adds unit tests for role extraction. Introduces a nil-check in the HTTP transport layer to prevent potential nil dereference.

Changes

Cohort / File(s)	Summary
Auth Logic Refactoring internal/auth/auth.go, internal/auth/auth_test.go	Consolidates role-detection logic into a roleFromGroups helper that parses groups with ganymede- or ganymede\_ prefixes. Applies role extraction to both user creation (with UserRole default) and updates (preserving existing roles if no valid group found). Adds comprehensive unit tests covering valid admin/editor groups and invalid group scenarios.
HTTP Transport Safety internal/transport/http/auth.go	Adds nil-check for the user returned by OAuthCallback to prevent nil dereference when accessing user.ID; returns internal server error if user is nil.

🚥 Pre-merge checks | ✅ 2 | ❌ 1

❌ Failed checks (1 warning)

Check name	Status	Explanation	Resolution
Docstring Coverage	⚠️ Warning	Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%.	Write docstrings for the functions missing them to satisfy the coverage threshold.

✅ Passed checks (2 passed)

Check name	Status	Explanation
Title check	✅ Passed	The title 'fix: prevent oauth nil user panic' clearly and specifically describes the main fix in the changeset - adding a nil-check for the OAuth user to prevent a panic.
Description check	✅ Passed	The description references issue #1051, which is directly related to the changeset focused on preventing an OAuth nil user panic.

_{✏️ Tip: You can configure your own custom pre-merge checks in the settings.}

✨ Finishing touches

• 📝 Generate docstrings

🧪 Generate unit tests (beta)

• Create PR with unit tests
• Post copyable unit tests in a comment
• Commit unit tests in branch oauth-nil-user-panic

---

No actionable comments were generated in the recent review. 🎉

🧹 Recent nitpick comments

internal/auth/auth_test.go (1)

10-34: Tests cover the core scenarios well; consider adding edge cases.

A couple of additional test cases would strengthen coverage:

• Empty []string{} input
• Multiple ganymede groups (e.g., ["ganymede-admin", "ganymede-editor"]) to document that the first match wins
• "ganymede-user" to confirm UserRole is also mapped correctly

These aren't blockers but would help document the function's behavior for future maintainers.

internal/auth/auth.go (1)

218-238: roleFromGroups returns the first matching group — document or handle multiple matches.

If a user belongs to both ganymede-admin and ganymede-editor, the result depends on the order the OIDC provider returns groups, which may not be deterministic. This is unlikely to cause issues in practice, but worth a brief code comment noting "first match wins" behavior.

Also, this would accept ganymede-user and map it to UserRole, which is technically valid per IsValidRole but could be surprising if the intent is that only elevated roles are managed via groups. If that's intentional, no change needed.

---

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands and usage tips.}