feat(model): add support for first and last name claims #69
Conversation
• Introduces new fields for first_name and last_name in the web token • Implements fallback logic to prioritize first_name/last_name over given_name/family_name
|
Caution Review failedThe pull request is closed. WalkthroughPopulate WebToken.FirstName and WebToken.LastName from raw claims (first_name/last_name with fallbacks to given_name/family_name), add RawGivenName/RawFamilyName and trimming logic, adjust id_token parse error wrapping, and add/rewire tests for success and error scenarios. Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related PRs
Suggested reviewers
Pre-merge checks and finishing touches✅ Passed checks (3 passed)
📜 Recent review detailsConfiguration used: CodeRabbit UI Review profile: CHILL Plan: Pro 📒 Files selected for processing (3)
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. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
jwt/model.go (1)
40-41: Do not include the raw id_token in errors/logs (PII leak).Leaking tokens can expose PII and credentials in logs. Remove the token string from the error.
Apply this diff:
- err = fmt.Errorf("unable to parse id_token: [%s], %w", idToken, parseErr) + err = fmt.Errorf("unable to parse id_token: %w", parseErr)Optionally add structured logging of safe metadata (e.g., error type) if needed.
🧹 Nitpick comments (3)
jwt/raw.go (1)
25-31: Treat whitespace-only names as empty to trigger fallback.If an IdP emits " " or "\t" for first/last name, current logic won’t fallback. Trim before checks.
Apply these diffs:
func (r rawWebToken) getLastName() (lastName string) { - lastName = r.LastName - if lastName == "" { - lastName = r.RawFamilyName + lastName = strings.TrimSpace(r.LastName) + if lastName == "" { + lastName = strings.TrimSpace(r.RawFamilyName) } return } func (r rawWebToken) getFirstName() (firstName string) { - firstName = r.FirstName - if firstName == "" { - firstName = r.RawGivenName + firstName = strings.TrimSpace(r.FirstName) + if firstName == "" { + firstName = strings.TrimSpace(r.RawGivenName) } return }Add import (outside this hunk):
import "strings"Also applies to: 33-39
jwt/raw_test.go (1)
41-129: Good coverage; consider table-driven tests and a whitespace case.
- Consolidate the six near-identical tests into table-driven subtests to reduce duplication.
- Add a case where first_name/last_name are " " to ensure trim-based fallback works if adopted.
If you want, I can draft the table-driven variant including a whitespace case.
jwt/model_test.go (1)
53-150: Tests clearly validate priority and fallbacks.Nice coverage across presence, preference, and partial fallback scenarios.
Add one more test with whitespace-only first_name/last_name to lock in the trimming behavior if implemented.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (4)
jwt/model.go(1 hunks)jwt/model_test.go(1 hunks)jwt/raw.go(2 hunks)jwt/raw_test.go(1 hunks)
🧰 Additional context used
🧬 Code graph analysis (2)
jwt/raw_test.go (1)
jwt/model.go (1)
UserAttributes(16-19)
jwt/model_test.go (1)
jwt/model.go (1)
New(37-59)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)
- GitHub Check: pipe / lint / lint
- GitHub Check: Analyze (go)
🔇 Additional comments (2)
jwt/raw.go (1)
4-8: Additions align with OIDC standard claims.Introducing RawGivenName/RawFamilyName with given_name/family_name tags is correct and non-breaking.
jwt/model.go (1)
55-56: LGTM: correctly maps normalized names onto WebToken.Assignments are after copying UserAttributes, so they properly override with preferred/fallback values.
Please confirm no callers relied on empty FirstName/LastName when given_name/family_name were present.
• Avoid printing PII information
• Ensures that name fields are clean and consistent • Prevents potential issues with empty or malformed names
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (5)
jwt/raw.go (5)
6-10: Support []string in audiences to match the struct comment.Comment says aud can be []string or string, but getAudiences only handles string and []interface{}. Add a []string branch.
Apply:
func (r rawWebToken) getAudiences() (audiences []string) { switch audienceList := r.RawAudiences.(type) { case string: audiences = []string{audienceList} + case []string: + audiences = append(audiences, audienceList...) case []interface{}: for _, val := range audienceList { aud, ok := val.(string) if ok { audiences = append(audiences, aud) } } } return }
19-25: Trim fallback email too.Fallback assigns RawEmail without trimming; whitespace-only RawEmail would leak through.
Apply:
func (r rawWebToken) getMail() (mail string) { - mail = strings.TrimSpace(r.RawMail) + mail = strings.TrimSpace(r.RawMail) if mail == "" { - mail = r.RawEmail + mail = strings.TrimSpace(r.RawEmail) } return }
27-33: Trim fallback family_name.Aligns with first pass trimming and avoids whitespace results.
Apply:
func (r rawWebToken) getLastName() (lastName string) { lastName = strings.TrimSpace(r.LastName) if lastName == "" { - lastName = r.RawFamilyName + lastName = strings.TrimSpace(r.RawFamilyName) } return }
35-41: Trim fallback given_name.Same rationale as last name.
Apply:
func (r rawWebToken) getFirstName() (firstName string) { firstName = strings.TrimSpace(r.FirstName) if firstName == "" { - firstName = r.RawGivenName + firstName = strings.TrimSpace(r.RawGivenName) } return }
20-20: DRY: helper for “first non-empty trimmed” selection.Reduces duplication across mail/first/last name getters.
Apply:
+// firstNonEmptyTrim returns the first non-empty trimmed string from vals. +func firstNonEmptyTrim(vals ...string) string { + for _, v := range vals { + if s := strings.TrimSpace(v); s != "" { + return s + } + } + return "" +} + -func (r rawWebToken) getMail() (mail string) { - mail = strings.TrimSpace(r.RawMail) - if mail == "" { - mail = strings.TrimSpace(r.RawEmail) - } - return -} +func (r rawWebToken) getMail() (mail string) { return firstNonEmptyTrim(r.RawMail, r.RawEmail) } -func (r rawWebToken) getLastName() (lastName string) { - lastName = strings.TrimSpace(r.LastName) - if lastName == "" { - lastName = strings.TrimSpace(r.RawFamilyName) - } - return -} +func (r rawWebToken) getLastName() (lastName string) { return firstNonEmptyTrim(r.LastName, r.RawFamilyName) } -func (r rawWebToken) getFirstName() (firstName string) { - firstName = strings.TrimSpace(r.FirstName) - if firstName == "" { - firstName = strings.TrimSpace(r.RawGivenName) - } - return -} +func (r rawWebToken) getFirstName() (firstName string) { return firstNonEmptyTrim(r.FirstName, r.RawGivenName) }Also applies to: 28-31, 36-39
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
jwt/model.go(2 hunks)jwt/raw.go(2 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- jwt/model.go
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)
- GitHub Check: Analyze (go)
- GitHub Check: pipe / lint / lint
🔇 Additional comments (1)
jwt/raw.go (1)
3-3: Import looks correct.Needed for TrimSpace; no issues.
• Refactor tests to use table-driven approach for better readability • Add comprehensive cases for first name, last name, and error handling
• Ensures consistent handling of user input for email and name fields
2 participants
Summary by CodeRabbit
New Features
Bug Fixes
Tests