[LFXV2-502] alternate email linking - authelia #17
Conversation
- Added a new KV bucket configuration for storing email OTPs in the Helm chart. - Implemented the email sending logic for OTP generation and verification in the Authelia flow. - Enhanced the user model to support alternate email linking and verification processes. - Updated the NATS storage to handle OTP creation and retrieval, ensuring secure storage and access. Jira Ticket: https://linuxfoundation.atlassian.net/browse/LFXV2-502 Generated with [Cursor](https://cursor.com/) Signed-off-by: Mauricio Zanetti Salomao <[email protected]>
- Introduced a new feature to retrieve user email addresses (primary and alternate) via the NATS subject `lfx.auth-service.user_emails.read`. - Updated the message handler to support user email retrieval operations. - Enhanced the README and added a new documentation file for user email operations, detailing request formats and response structures. - Incremented the chart version to 0.3.1 to reflect the new feature addition. Jira Ticket: https://linuxfoundation.atlassian.net/browse/LFXV2-501 Generated with [Cursor](https://cursor.com/) Signed-off-by: Mauricio Zanetti Salomao <[email protected]>
- Introduced a new section in the README detailing the OTP verification process for linking alternate email addresses to user accounts. - Documented the architecture, flow, and storage implementation of the OTP system, including NATS KV bucket configuration and token generation. - Enhanced integration details with the identity linking system, outlining the steps for verification and linking phases. Jira Ticket: https://linuxfoundation.atlassian.net/browse/LFXV2-502 Generated with [Cursor](https://cursor.com/) Signed-off-by: Mauricio Zanetti Salomao <[email protected]>
|
Note Other AI code review bot(s) detectedCodeRabbit has detected other AI code review bot(s) in this pull request and will avoid duplicating their findings in the review comments. This may lead to a less comprehensive review. WalkthroughAdds user-email retrieval and alternate-email OTP flows: domain model updates (primary/alternate emails), new NATS subject/handler, Authelia OTP KV bucket and storage revision support, SMTP email sender and EmailSender port, JWT generator/parser additions, Helm/values and documentation updates. Changes
Sequence Diagram(s)sequenceDiagram
autonumber
participant Client
participant NATS
participant Handler as MessageHandler
participant Reader as UserReader
participant Storage
Client->>NATS: Publish `lfx.auth-service.user_emails.read`
NATS->>Handler: Route message
Handler->>Handler: Parse input (JWT / subject / username)
Handler->>Reader: getUserByInput(...)
Reader->>Storage: Lookup by index (email/sub/username)
Storage-->>Reader: Return user (+ revision)
Reader-->>Handler: User payload
Handler->>NATS: Reply { primary_email, alternate_emails }
NATS-->>Client: Response
sequenceDiagram
autonumber
participant Caller
participant Service as AutheliaUserService
participant EmailFlow as autheliaPasswordlessFlow
participant SMTP
participant KV as NATS-KV (otp bucket)
Caller->>Service: SendVerificationAlternateEmail(email)
Service->>EmailFlow: SendEmail(email)
EmailFlow->>EmailFlow: Generate 6-digit OTP
EmailFlow->>SMTP: sendEmail(EmailMessage)
SMTP-->>EmailFlow: delivery result
EmailFlow-->>Service: OTP
Service->>KV: CreateVerificationCode(email, otp) with TTL
KV-->>Service: ack
Service-->>Caller: success
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Areas to focus review on:
Possibly related PRs
Pre-merge checks and finishing touches❌ Failed checks (1 warning, 1 inconclusive)
✅ Passed checks (2 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
📜 Recent review detailsConfiguration used: CodeRabbit UI Review profile: CHILL Plan: Pro Disabled knowledge base sources:
📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
⏰ 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). (1)
Comment |
There was a problem hiding this comment.
Pull Request Overview
This PR implements alternate email linking functionality for the Authelia authentication provider through a passwordless OTP verification flow. The implementation brings Authelia to feature parity with Auth0 for email verification and identity linking workflows.
- OTP-based email verification system with 6-digit codes sent via SMTP
- JWT identity token generation for verified emails with 60-minute expiration
- Identity linking to user accounts with optimistic locking for concurrent update safety
Reviewed Changes
Copilot reviewed 37 out of 37 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
| pkg/password/generate.go | Added OnlyNumbers function for generating numeric OTP codes |
| pkg/password/generate_test.go | Comprehensive test coverage for password generation functions |
| pkg/jwt/generator.go | New JWT generation utilities supporting access and identity tokens |
| pkg/jwt/generator_test.go | Test suite for JWT generation functions |
| pkg/jwt/parser.go | Added email claim extraction support |
| pkg/jwt/README.md | Documentation for JWT generation and parsing utilities |
| pkg/constants/subjects.go | Added NATS subjects for email operations and identity linking |
| pkg/constants/storage.go | Added KV bucket constant for email OTP storage |
| pkg/constants/global.go | Added SMTP configuration environment variable keys |
| internal/service/message_handler.go | Implemented GetUserEmails handler and updated LinkIdentity flow |
| internal/infrastructure/smtp/sender.go | SMTP email sender implementation |
| internal/infrastructure/smtp/client.go | SMTP client with environment-based configuration |
| internal/infrastructure/nats/client.go | Added email OTP bucket to NATS client initialization |
| internal/infrastructure/authelia/user.go | Implemented email verification and identity linking for Authelia |
| internal/infrastructure/authelia/storage.go | Added OTP storage and optimistic locking support |
| internal/infrastructure/authelia/models.go | Updated models to support alternate emails |
| internal/infrastructure/authelia/email.go | Passwordless email flow implementation |
| internal/infrastructure/authelia/sync_test.go | Updated mock storage for new methods |
| internal/infrastructure/authelia/README.md | Documentation for OTP verification flow |
| internal/infrastructure/auth0/user.go | Refactored LinkIdentity to use user_id from request |
| internal/infrastructure/auth0/models.go | Updated field names for consistency (AlternateEmail → AlternateEmails) |
| internal/infrastructure/auth0/filter.go | Updated to use renamed AlternateEmails field |
| internal/infrastructure/auth0/filter_test.go | Updated tests for field name changes |
| internal/domain/port/message_handler.go | Added GetUserEmails to handler interface |
| internal/domain/port/email_sender.go | New port interface for email sending |
| internal/domain/model/user.go | Renamed AlternateEmail to AlternateEmails, added index key builder |
| internal/domain/model/identity.go | Added UserID field to LinkIdentity request |
| internal/domain/model/email.go | Renamed EmailVerified to Verified, added EmailMessage model |
| internal/domain/model/email_test.go | Updated tests and added EmailMessage validation tests |
| docs/user_emails.md | New documentation for user emails retrieval |
| docs/identity_linking.md | Updated documentation for new request structure |
| cmd/server/service/providers.go | Added UserEmailReadSubject subscription |
| cmd/server/service/message_handler.go | Added GetUserEmails handler mapping |
| charts/lfx-v2-auth-service/values.yaml | Added email OTP KV bucket configuration |
| charts/lfx-v2-auth-service/templates/nats-kv-buckets.yaml | Added email OTP KV bucket template |
| charts/lfx-v2-auth-service/Chart.yaml | Version bump to 0.3.1 |
| README.md | Added user emails operations documentation link |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
mauriciozanettisalomao
changed the title
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 12
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
internal/infrastructure/nats/client.go (1)
58-71: Avoid logging raw NATS URLs; potential secret leakage.ConnectedUrl can include credentials in some setups. Drop it or redact.
Apply this diff to remove URL logging:
- slog.ErrorContext(ctx, "error creating NATS JetStream client", - "error", err, - "nats_url", c.conn.ConnectedUrl(), - ) + slog.ErrorContext(ctx, "error creating NATS JetStream client", + "error", err, + ) @@ - slog.ErrorContext(ctx, "error getting NATS JetStream key-value store", - "error", err, - "nats_url", c.conn.ConnectedUrl(), - "bucket", bucketName, - ) + slog.ErrorContext(ctx, "error getting NATS JetStream key-value store", + "error", err, + "bucket", bucketName, + ) @@ - slog.InfoContext(ctx, "NATS client created successfully", - "connected_url", conn.ConnectedUrl(), - "status", conn.Status(), - ) + slog.InfoContext(ctx, "NATS client created successfully", + "status", conn.Status(), + )Also applies to: 182-185
🧹 Nitpick comments (31)
pkg/password/generate.go (2)
41-50: Micro‑opt: reuse the RNG bound.
Avoid reallocating big.NewInt in the loop; hoist it once.const charset = "0123456789" result := make([]byte, length) +max := big.NewInt(int64(len(charset))) + for i := range result { - num, err := rand.Int(rand.Reader, big.NewInt(int64(len(charset)))) + num, err := rand.Int(rand.Reader, max) if err != nil { return "", err } result[i] = charset[num.Int64()] }
36-36: Optional naming nit.
Consider Digits or Numeric for consistency with AlphaNum. Public rename is breaking; defer if API stability is required.pkg/password/generate_test.go (4)
13-77: Good coverage; consider asserting error type.
Tests are solid. Optionally assert that invalid lengths return a pkg/errors.Validation to lock the contract; also consider compiling the regex once with MustCompile.
100-164: OnlyNumbers tests look good; same optional hardening.
Optionally assert specific error type on invalid lengths and precompile the regex pattern once.
187-267: Avoid brittle bcrypt prefix checks.
Prefix can vary across implementations; rely on bcrypt.Cost (or widen regex) instead.- // Check that the hash starts with bcrypt prefix - if len(bcryptHash) < 7 || bcryptHash[:4] != "$2a$" && bcryptHash[:4] != "$2b$" { - t.Errorf("bcrypt hash doesn't have expected format: %s", bcryptHash) - } + // Validate hash format via bcrypt parser + if _, err := bcrypt.Cost([]byte(bcryptHash)); err != nil { + t.Errorf("bcrypt hash doesn't have expected bcrypt format: %v", err) + }
269-296: Uniqueness test LGTM; consider -short guard.
bcrypt can be slow on CI; optionally skip this in short mode:
if testing.Short() { t.Skip("skipping slow bcrypt uniqueness test") }internal/infrastructure/nats/client.go (1)
162-167: Prefer config over env for provider detection.Reading USER_REPOSITORY_TYPE directly hinders testability. Consider surfacing this via Config and letting env→config happen at composition.
If you keep env, ensure values are documented and validated at startup.
pkg/constants/global.go (1)
68-87: SMTP config keys LGTM; consider TLS flags and secret handling.Add optional flags like EMAIL_SMTP_STARTTLS, EMAIL_SMTP_TLS, EMAIL_SMTP_TLS_SKIP_VERIFY (dev only), and never log EMAIL_SMTP_PASSWORD.
Confirm smtp client enforces TLS by default and redacts creds in logs.
internal/infrastructure/authelia/email.go (2)
6-17: Source From/FromName from env (avoid hard-coded sender).Use the new constants so deploys can set the sender identity without code changes.
Apply this diff:
@@ -import ( +import ( "context" "fmt" "log/slog" + "os" @@ - "github.com/linuxfoundation/lfx-v2-auth-service/pkg/errors" + "github.com/linuxfoundation/lfx-v2-auth-service/pkg/errors" + "github.com/linuxfoundation/lfx-v2-auth-service/pkg/constants" @@ - message := &model.EmailMessage{ - From: "[email protected]", - To: email, - Subject: "Welcome to Linux Foundation", - Body: fmt.Sprintf("Your verification code is: %s", otp), - } + from := os.Getenv(constants.EmailFromAddressEnvKey) + if from == "" { + from = "[email protected]" + } + fromName := os.Getenv(constants.EmailFromNameEnvKey) + message := &model.EmailMessage{ + From: from, + FromName: fromName, + To: email, + Subject: "Your LFX verification code", + Body: fmt.Sprintf("Your verification code is: %s", otp), + }Also applies to: 36-41
63-67: Allow dependency injection for sender (testability).Accept port.EmailSender in constructor; keep smtp.NewSender() as default path in wiring, not here.
Example:
-func newEmailLinkingFlow() passwordlessFlow { - return &autheliaPasswordlessFlow{ - emailSender: smtp.NewSender(), - } -} +func newEmailLinkingFlowWith(sender port.EmailSender) passwordlessFlow { + return &autheliaPasswordlessFlow{ emailSender: sender } +} +// production wiring: +func newEmailLinkingFlow() passwordlessFlow { return newEmailLinkingFlowWith(smtp.NewSender()) }internal/domain/model/user.go (2)
65-80: Sanitize AlternateEmails too.Trim email strings to avoid index/key mismatches and noisy diffs.
Apply this diff:
func (u *User) UserSanitize() { @@ u.PrimaryEmail = strings.TrimSpace(u.PrimaryEmail) @@ if u.UserMetadata != nil { u.UserMetadata.userMetadataSanitize() } + + // Sanitize alternate emails + for i := range u.AlternateEmails { + u.AlternateEmails[i].Email = strings.TrimSpace(u.AlternateEmails[i].Email) + }
82-95: Use email-specific redaction when logging email-kind indices.Avoid over/under-redaction by using RedactEmail for email and alternate-email kinds.
Apply this diff:
func (u User) buildIndexKey(ctx context.Context, kind, data string) string { @@ - slog.DebugContext(ctx, "index key built", - "kind", kind, - "data", redaction.Redact(data), - "key", key, - ) + redacted := redaction.Redact(data) + if kind == "email" || kind == "alternate-email" { + redacted = redaction.RedactEmail(data) + } + slog.DebugContext(ctx, "index key built", + "kind", kind, + "data", redacted, + "key", key, + )Based on learnings.
pkg/jwt/parser.go (1)
251-251: Nice refactor using maps.Copy.Replacing manual iteration with
maps.Copyfor copying private claims is cleaner and more idiomatic.docs/user_emails.md (1)
29-41: Add language identifiers to fenced code blocks.The fenced code blocks on lines 29, 34, and 39 are missing language identifiers, which affects proper syntax highlighting and violates markdown linting rules.
Apply this diff to add language identifiers:
**JWT Token (Auth0):** -``` +```text eyJhbGciOiJSUzI1NiIsInR5cCI6IkpXVCJ9...Subject Identifier:
-+text
auth0|123456789**Username:** -``` +```text john.doe</blockquote></details> <details> <summary>internal/infrastructure/smtp/sender.go (1)</summary><blockquote> `58-63`: **Consider redacting the recipient email from debug logs.** Based on learnings from previous reviews, sensitive user information including emails should be redacted from logs. While this is a debug log, the recipient email address is PII and should be masked or excluded. Based on learnings Apply this diff to redact the recipient: ```diff slog.DebugContext(ctx, "email sent successfully via SMTP", "host", s.client.config.Host, "port", s.client.config.Port, - "to", message.To, + "to", "[REDACTED]", "subject", message.Subject, )internal/infrastructure/auth0/filter_test.go (2)
645-661: Test asserts mutation of source DTO; confirm intended contractThe test expects alternateEmailFilter.Filter to append to auth0User.AlternateEmail. If the filter is intended to be a pure predicate that only mutates the domain user, this couples tests to internal behavior and may break easily. Consider asserting user-side effects (e.g., updated user.AlternateEmails) or returned flags, not DTO mutation.
539-559: Clarify Verified semantics in matchingThis case matches even when the local alternate email is Verified=false. If the filter should only match verified alternates, adjust the expected result; if it deliberately ignores local Verified (trusting identity.EmailVerified), add a brief comment to prevent confusion.
internal/infrastructure/auth0/models.go (1)
84-91: Avoid variable shadowing and preallocate sliceMinor nit: alternateEmail variable is shadowed inside the loop; also preallocate capacity.
- var alternateEmails []model.Email - for _, alternateEmail := range u.AlternateEmail { - alternateEmail := model.Email{ - Email: alternateEmail.Email, - Verified: alternateEmail.EmailVerified, - } - alternateEmails = append(alternateEmails, alternateEmail) - } + alternateEmails := make([]model.Email, 0, len(u.AlternateEmail)) + for _, ae := range u.AlternateEmail { + alt := model.Email{ + Email: ae.Email, + Verified: ae.EmailVerified, + } + alternateEmails = append(alternateEmails, alt) + }internal/service/message_handler.go (1)
93-101: Comment contradicts implementationComment claims alternate email search is not available, but code handles it. Update the comment to reflect actual behavior to avoid confusion.
internal/infrastructure/authelia/storage.go (6)
230-246: Use validation errors for missing inputsCreation of verification codes with empty email/otp should return client validation errors, not unexpected server errors.
- if email == "" { - return errs.NewUnexpected("email is required") - } - if otp == "" { - return errs.NewUnexpected("otp is required") - } + if email == "" { + return errs.NewValidation("email is required") + } + if otp == "" { + return errs.NewValidation("otp is required") + }
254-267: Return validation error on empty key inputSame here for GetVerificationCode: empty input should be a validation error.
- if email == "" { - return "", errs.NewUnexpected("email is required") - } + if email == "" { + return "", errs.NewValidation("email is required") + }
284-301: Include bucket name in error for easier ops debuggingWhen a KV bucket is missing, include which one failed.
- if !exists { - return nil, errs.NewUnexpected("KV bucket not found in NATS client") - } + if !exists { + return nil, errs.NewUnexpected(fmt.Sprintf("KV bucket %q not found in NATS client", bucketName)) + }
138-161: Lookup key maintenance: consider removing stale keyssetLookupKeys adds email/alt/sub lookup keys but doesn’t remove stale ones when emails are removed or change, leaving dangling indices. Consider reconciling keys (diff old vs new and delete removed) after updates.
Would you like a follow-up change to implement cleanup in UpdateUserWithRevision?
112-136: Keys() error handling is brittleRelying on substring "no keys found" is fragile. If the client exposes a sentinel error, prefer that; otherwise, treat ErrKeyNotFound consistently or handle nil/empty list without logging errors.
If jetstream provides a typed error for the "no keys" case, we can switch to it.
230-276: Param naming/logging clarity for OTP keyCreate/GetVerificationCode take an argument named email but the caller passes a lookup key (BuildAlternateEmailIndexKey). Rename the param to indexKey and redact logs appropriately if you ever pass raw emails.
Based on learningspkg/jwt/generator.go (6)
6-15: Add missing imports for new validations and KID support.You’ll need fmt and jws for the other suggested diffs.
Apply this diff:
@@ -import ( +import ( "crypto/rand" "crypto/rsa" "sync" "time" + "fmt" "github.com/lestrrat-go/jwx/v2/jwa" + "github.com/lestrrat-go/jwx/v2/jws" "github.com/lestrrat-go/jwx/v2/jwt" "github.com/linuxfoundation/lfx-v2-auth-service/pkg/errors" )
57-83: Support JWT header kid.Add KeyID to options and emit it in the signature header for key rotation.
Apply these diffs:
@@ type GeneratorOptions struct { // SigningKey is the key used to sign the token (RSA private key or HMAC secret) SigningKey any + // KeyID sets the 'kid' header on the JWS for key identification/rotation + KeyID string }@@ - // Sign the token - signed, err := jwt.Sign(token, jwt.WithKey(opts.SigningMethod, opts.SigningKey)) + // Sign the token (attach kid when provided) + var signOpt jwt.SignOption + if opts.KeyID != "" { + signOpt = jwt.WithKey(opts.SigningMethod, opts.SigningKey, jws.WithKeyID(opts.KeyID)) + } else { + signOpt = jwt.WithKey(opts.SigningMethod, opts.SigningKey) + } + signed, err := jwt.Sign(token, signOpt) if err != nil { return "", errors.NewUnexpected("failed to sign JWT token", err) }Also applies to: 222-226
148-151: Validate alg vs key type early.Fail fast with clear messages when alg/key type mismatch instead of relying on downstream errors.
Apply this snippet after the SigningKey nil check:
@@ if opts.SigningKey == nil { return "", errors.NewValidation("signing key is required") } + switch opts.SigningMethod { + case jwa.RS256: + if _, ok := opts.SigningKey.(*rsa.PrivateKey); !ok { + return "", errors.NewValidation("RS256 requires *rsa.PrivateKey signing key") + } + case jwa.HS256: + if _, ok := opts.SigningKey.([]byte); !ok { + return "", errors.NewValidation("HS256 requires []byte HMAC secret") + } + }
169-174: Use UTC timestamps to avoid TZ-related validation issues.Prefer time.Now().UTC() for iat/exp defaults across constructors and helpers.
Minimal example:
- IssuedAt: time.Now(), + IssuedAt: time.Now().UTC(),Repeat similarly where time.Now() is used in this file.
Also applies to: 333-341, 241-243, 255-258, 271-274, 286-289, 300-302, 312-313, 370-373
190-193: Audience semantics.If multiple audiences are expected, consider changing Audience to []string in options and builder to pass as-is. If single-audience only, ignore.
No code change required if single aud is guaranteed.
336-351: Use RFC 2606 reserved domains in test helpers.Swap test.any.com for example.com or example.test to avoid accidental DNS resolution.
Example:
- "https://test.any.com/", - "https://test.any.com/api/v2/", + "https://example.com/", + "https://example.com/api/v2/",
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
Disabled knowledge base sources:
- Jira integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (37)
README.md(1 hunks)charts/lfx-v2-auth-service/Chart.yaml(1 hunks)charts/lfx-v2-auth-service/templates/nats-kv-buckets.yaml(2 hunks)charts/lfx-v2-auth-service/values.yaml(1 hunks)cmd/server/service/message_handler.go(1 hunks)cmd/server/service/providers.go(1 hunks)docs/identity_linking.md(2 hunks)docs/user_emails.md(1 hunks)internal/domain/model/email.go(2 hunks)internal/domain/model/email_test.go(2 hunks)internal/domain/model/identity.go(1 hunks)internal/domain/model/user.go(2 hunks)internal/domain/port/email_sender.go(1 hunks)internal/domain/port/message_handler.go(1 hunks)internal/infrastructure/auth0/filter.go(1 hunks)internal/infrastructure/auth0/filter_test.go(11 hunks)internal/infrastructure/auth0/models.go(1 hunks)internal/infrastructure/auth0/user.go(3 hunks)internal/infrastructure/authelia/README.md(1 hunks)internal/infrastructure/authelia/email.go(1 hunks)internal/infrastructure/authelia/models.go(3 hunks)internal/infrastructure/authelia/storage.go(6 hunks)internal/infrastructure/authelia/sync_test.go(1 hunks)internal/infrastructure/authelia/user.go(11 hunks)internal/infrastructure/nats/client.go(1 hunks)internal/infrastructure/smtp/client.go(1 hunks)internal/infrastructure/smtp/sender.go(1 hunks)internal/service/message_handler.go(7 hunks)pkg/constants/global.go(1 hunks)pkg/constants/storage.go(1 hunks)pkg/constants/subjects.go(3 hunks)pkg/jwt/README.md(1 hunks)pkg/jwt/generator.go(1 hunks)pkg/jwt/generator_test.go(1 hunks)pkg/jwt/parser.go(4 hunks)pkg/password/generate.go(1 hunks)pkg/password/generate_test.go(1 hunks)
🧰 Additional context used
🧠 Learnings (3)
📓 Common learnings
Learnt from: mauriciozanettisalomao
PR: linuxfoundation/lfx-v2-auth-service#2
File: internal/infrastructure/auth0/user.go:16-21
Timestamp: 2025-09-19T17:04:25.227Z
Learning: In the LFX v2 Auth Service Auth0 implementation, mauriciozanettisalomao plans to redact sensitive user information (tokens, emails, metadata) from logs and use appropriate log levels during the actual Auth0 and Authelia implementation, rather than in the current stub/placeholder code.
📚 Learning: 2025-10-23T13:31:11.681Z
Learnt from: mauriciozanettisalomao
PR: linuxfoundation/lfx-v2-auth-service#16
File: internal/infrastructure/auth0/user.go:57-59
Timestamp: 2025-10-23T13:31:11.681Z
Learning: In internal/infrastructure/auth0/user.go, the filterer.Args(ctx) method returns already URL-encoded arguments. Each filter implementation (usernameFilter, emailFilter, alternateEmailFilter) handles URL encoding within its Args() method using url.QueryEscape, so no additional encoding is needed at the call site in SearchUser.
Applied to files:
internal/infrastructure/auth0/filter.go
📚 Learning: 2025-09-19T17:05:21.230Z
Learnt from: mauriciozanettisalomao
PR: linuxfoundation/lfx-v2-auth-service#2
File: internal/infrastructure/mock/user.go:47-69
Timestamp: 2025-09-19T17:05:21.230Z
Learning: The mock user implementation in internal/infrastructure/mock/user.go uses fantasy/fake user data (like "zephyr.stormwind", "aurora.moonbeam") for development and testing purposes, so logging full user objects is acceptable in this context.
Applied to files:
internal/infrastructure/authelia/sync_test.go
🧬 Code graph analysis (24)
internal/infrastructure/nats/client.go (1)
pkg/constants/storage.go (1)
KVBucketNameAutheliaEmailOTP(12-12)
cmd/server/service/providers.go (1)
pkg/constants/subjects.go (1)
UserEmailReadSubject(39-39)
internal/domain/port/email_sender.go (1)
internal/domain/model/email.go (1)
EmailMessage(25-38)
internal/infrastructure/smtp/client.go (2)
pkg/errors/server.go (1)
NewUnexpected(19-26)pkg/constants/global.go (4)
EmailSMTPHostEnvKey(71-71)EmailSMTPPortEnvKey(74-74)EmailSMTPUsernameEnvKey(83-83)EmailSMTPPasswordEnvKey(86-86)
internal/infrastructure/auth0/filter.go (2)
internal/domain/model/email.go (1)
internal/infrastructure/auth0/models.go (1)
Auth0User(14-24)
pkg/password/generate.go (1)
pkg/errors/client.go (1)
NewValidation(19-26)
cmd/server/service/message_handler.go (1)
pkg/constants/subjects.go (1)
UserEmailReadSubject(39-39)
internal/domain/model/email_test.go (1)
internal/domain/model/email.go (2)
EmailMessage(25-38)
internal/infrastructure/smtp/sender.go (3)
internal/domain/model/email.go (1)
EmailMessage(25-38)pkg/errors/client.go (1)
NewValidation(19-26)internal/domain/port/email_sender.go (1)
EmailSender(13-16)
internal/infrastructure/authelia/models.go (2)
internal/domain/model/email.go (1)
internal/domain/model/user.go (2)
UserMetadata(30-45)User(19-27)
pkg/jwt/generator_test.go (2)
pkg/jwt/generator.go (16)
GeneratorOptions(58-83)TokenType(48-48)TokenTypeAccess(52-52)Generate(143-229)TokenTypeIdentity(54-54)GenerateAccessToken(232-245)GenerateIdentityToken(248-260)GenerateTestAccessToken(296-302)GetDefaultTestPublicKey(39-45)GenerateTestIdentityToken(307-313)GenerateTestHMACIdentityToken(325-327)GenerateSimpleTestAccessToken(332-340)GenerateSimpleTestIdentityToken(345-352)IdentityTokenOptions(107-116)HMACIdentityTokenOptions(131-140)DefaultGeneratorOptions(86-92)pkg/jwt/parser.go (4)
ParseOptions(37-54)ParseVerified(128-202)ParseUnverified(68-124)DefaultParseOptions(57-63)
internal/infrastructure/auth0/user.go (3)
internal/domain/model/user.go (1)
User(19-27)pkg/errors/client.go (1)
NewValidation(19-26)pkg/redaction/redaction.go (1)
Redact(22-42)
internal/infrastructure/authelia/user.go (13)
internal/infrastructure/authelia/models.go (1)
OIDCUserInfo(13-21)pkg/errors/client.go (1)
NewValidation(19-26)pkg/constants/user.go (1)
CriteriaTypeAlternateEmail(12-12)pkg/redaction/redaction.go (2)
RedactEmail(54-72)Redact(22-42)internal/domain/model/email.go (1)
pkg/errors/server.go (1)
NewUnexpected(19-26)internal/domain/model/user.go (1)
User(19-27)internal/domain/model/auth.go (1)
AuthResponse(8-14)pkg/jwt/generator.go (3)
GenerateSimpleTestIdentityTokenWithSubject(357-375)GenerateSimpleTestAccessToken(332-340)TokenType(48-48)pkg/jwt/parser.go (2)
ParseOptions(37-54)ParseUnverified(68-124)internal/infrastructure/auth0/user.go (1)
NewUserReaderWriter(389-433)internal/domain/port/user.go (1)
UserReaderWriter(13-18)pkg/httpclient/config.go (1)
DefaultConfig(26-33)
internal/domain/port/message_handler.go (1)
internal/domain/port/transport_messenger.go (1)
TransportMessenger(7-11)
internal/infrastructure/authelia/email.go (6)
internal/domain/port/email_sender.go (1)
EmailSender(13-16)pkg/password/generate.go (1)
OnlyNumbers(36-53)pkg/errors/server.go (1)
NewUnexpected(19-26)internal/domain/model/email.go (1)
EmailMessage(25-38)pkg/redaction/redaction.go (1)
RedactEmail(54-72)internal/infrastructure/smtp/sender.go (1)
NewSender(69-73)
pkg/jwt/parser.go (1)
internal/domain/model/email.go (1)
internal/infrastructure/auth0/filter_test.go (2)
internal/domain/model/email.go (1)
internal/domain/model/user.go (1)
User(19-27)
internal/infrastructure/authelia/storage.go (5)
internal/infrastructure/authelia/models.go (2)
AutheliaUser(24-37)AutheliaUserStorage(41-50)internal/infrastructure/nats/client.go (1)
NATSClient(21-26)pkg/constants/storage.go (2)
KVBucketNameAutheliaUsers(9-9)KVBucketNameAutheliaEmailOTP(12-12)pkg/errors/server.go (1)
NewUnexpected(19-26)pkg/errors/client.go (2)
NewNotFound(79-86)NewConflict(99-106)
pkg/password/generate_test.go (1)
pkg/password/generate.go (3)
AlphaNum(15-33)OnlyNumbers(36-53)GeneratePasswordPair(56-72)
internal/infrastructure/authelia/sync_test.go (1)
internal/infrastructure/authelia/models.go (1)
AutheliaUser(24-37)
internal/service/message_handler.go (6)
internal/domain/model/email.go (1)
internal/domain/port/transport_messenger.go (1)
TransportMessenger(7-11)internal/domain/model/user.go (1)
User(19-27)pkg/errors/server.go (1)
NewUnexpected(19-26)pkg/errors/client.go (1)
NewValidation(19-26)pkg/redaction/redaction.go (1)
Redact(22-42)
internal/domain/model/user.go (1)
internal/domain/model/email.go (1)
internal/infrastructure/auth0/models.go (2)
internal/domain/model/email.go (1)
internal/domain/model/user.go (2)
User(19-27)UserMetadata(30-45)
pkg/jwt/generator.go (2)
pkg/errors/client.go (1)
NewValidation(19-26)pkg/errors/server.go (1)
NewUnexpected(19-26)
🪛 Gitleaks (8.28.0)
docs/identity_linking.md
[high] 21-21: Detected a Generic API Key, potentially exposing access to various services and sensitive operations.
(generic-api-key)
[high] 24-24: Detected a Generic API Key, potentially exposing access to various services and sensitive operations.
(generic-api-key)
[high] 68-68: Detected a Generic API Key, potentially exposing access to various services and sensitive operations.
(generic-api-key)
[high] 71-71: Detected a Generic API Key, potentially exposing access to various services and sensitive operations.
(generic-api-key)
🪛 LanguageTool
pkg/jwt/README.md
[style] ~397-~397: Using many exclamation marks might seem excessive (in this case: 4 exclamation marks for a text that’s 2695 characters long)
Context: ...efault test methods are for testing only!** - Keys are generated once and reused ...
(EN_EXCESSIVE_EXCLAMATION)
🪛 markdownlint-cli2 (0.18.1)
docs/user_emails.md
29-29: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
34-34: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
39-39: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
pkg/jwt/README.md
30-30: Spaces inside emphasis markers
(MD037, no-space-in-emphasis)
⏰ 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). (1)
- GitHub Check: MegaLinter
🔇 Additional comments (34)
pkg/password/generate_test.go (3)
79-99: Randomness check looks fine.
Collision probability here is negligible; test reads well.
166-185: Randomness check is reasonable.
Allowing up to two collisions keeps flake risk very low.
298-313: Cost verification LGTM.
Clear assertion against bcrypt.DefaultCost.internal/domain/model/user.go (1)
47-63: Validate() requires UserMetadata; confirm this is intentional.Many flows may not supply metadata for read-only operations. If not required everywhere, consider relaxing or validating per-operation instead.
internal/domain/port/message_handler.go (1)
16-18: I need to verify whether implementers of these interfaces exist and if a backward-compatibility alias is actually necessary.Now I need to verify the core assumption in the review comment—whether
UserReadHandleractually exists or was renamed:Based on my investigation, I found a critical issue with the original review comment:
The review comment assumes an interface rename from
UserReadHandlertoUserReaderHandler, but the current code shows only new interface names with no evidence of the old name existing.The script output revealed:
- No references to
UserReadHandleranywhere in the codebase- Only the new interface names (
UserReaderHandler,UserLookupHandler,UserLinkHandler) are present- No implementers of these interfaces were found
Type aliases are intended to support gradual code repair while moving a type between packages during large-scale refactoring, but only when there is an actual rename or migration occurring. If these are newly introduced interfaces without existing implementers of an old name, the suggested alias would be unnecessary.
The interfaces shown in lines 16–18 appear to be new additions rather than renames. Without evidence of
UserReadHandlerexisting in the codebase or an actual breaking change to existing implementers, the backward-compatibility alias suggestion is not applicable to this PR.Likely an incorrect or invalid review comment.
cmd/server/service/providers.go (1)
215-215: LGTM!The new subscription entry for
UserEmailReadSubjectfollows the established pattern and correctly wires the subject to the message handler.cmd/server/service/message_handler.go (1)
32-32: LGTM!The routing entry correctly maps
UserEmailReadSubjectto theGetUserEmailshandler and is appropriately placed in the user read/write operations section.charts/lfx-v2-auth-service/Chart.yaml (1)
8-8: LGTM!The patch version increment is appropriate for the new email OTP KV bucket feature and follows the project's versioning conventions.
pkg/constants/storage.go (1)
11-12: LGTM!The new constant for the Authelia Email OTP bucket follows naming conventions and is well-documented.
internal/infrastructure/auth0/filter.go (2)
124-127: LGTM!The bounds check before accessing the first alternate email is correct, and the URL encoding is properly handled per the learning that each filter implementation handles encoding within its
Args()method.
133-147: LGTM!The migration from singular
AlternateEmailto pluralAlternateEmailsis consistent, and the iteration logic correctly processes the slice of alternate emails.pkg/jwt/parser.go (2)
26-26: LGTM!The
Claimsstruct appropriately extends the public API to support email extraction from JWT tokens.
220-225: LGTM!The email extraction logic safely handles the type assertion and correctly populates the
README.md (1)
84-92: LGTM!The new documentation section clearly describes the User Emails Operations feature and appropriately notes the Authelia-only support limitation.
internal/domain/model/identity.go (1)
10-11: LGTM!The
UserIDfield addition properly extends theLinkIdentitystruct to support identifying the target user during link operations, with appropriate documentation and JSON tagging.internal/domain/port/email_sender.go (1)
12-16: LGTM!The
EmailSenderinterface is well-designed with a clean, single-responsibility contract for email operations. The context parameter enables proper cancellation and timeout handling.internal/domain/model/email_test.go (1)
248-360: Comprehensive test coverage for EmailMessage validation.The test suite thoroughly validates
EmailMessage.IsValid()across multiple scenarios including required fields, optional fields, email format validation, and HTML content support. The test cases are well-structured and cover both valid and invalid inputs.docs/user_emails.md (1)
1-190: Comprehensive and well-structured documentation.The documentation provides clear guidance on the user emails retrieval feature with excellent examples, use cases, and detailed field descriptions. The hybrid input approach is well-explained with practical CLI examples.
charts/lfx-v2-auth-service/templates/nats-kv-buckets.yaml (1)
24-43: Well-structured KV bucket for OTP storage.The new
authelia_email_otp_kv_bucketresource is properly configured with TTL support for temporary OTP codes. The structure mirrors the existing user bucket and includes appropriate lifecycle management with the optional "keep" annotation.internal/infrastructure/authelia/README.md (1)
182-290: Excellent documentation for OTP verification flow.The documentation provides comprehensive coverage of the alternate email linking feature with clear sequence diagrams, storage implementation details, and security considerations. The flow is well-explained with proper references to related documentation.
docs/identity_linking.md (1)
19-26: Improved payload structure with nested organization.The migration to a nested payload structure (
user.auth_tokenandlink_with.identity_token) improves clarity and organization compared to the flat structure. This change is consistently applied throughout all examples and documentation.internal/infrastructure/auth0/user.go (2)
355-357: Good validation: Ensuring UserID is provided.The explicit validation for
request.User.UserIDensures the required field is present before proceeding with the identity linking operation. This is clearer than the previous JWT extraction approach.
367-383: Simplified flow using explicit UserID.The change to use the explicitly provided
request.User.UserIDinstead of extracting it from JWT claims simplifies the flow and makes the contract clearer. The user_id is properly redacted in logs for security.Based on learnings
internal/infrastructure/smtp/client.go (2)
34-54: LGTM!The error handling and logging are appropriate. Sensitive data (credentials, recipient email) are properly excluded from logs, while infrastructure details (host, port) provide useful debugging context.
56-75: LGTM!The configuration defaults are appropriate for local development, and the optional authentication handling is correct.
internal/infrastructure/authelia/models.go (2)
61-83: LGTM!The AlternateEmail field mapping is correct. The naming difference (AlternateEmails in User vs AlternateEmail in Storage) is consistent with the domain model design shown in related files.
87-103: LGTM!The field mappings from storage to user model are correct and consistent with the ToStorage direction.
internal/infrastructure/smtp/sender.go (1)
68-73: LGTM!The factory function is clean and properly returns the interface type.
pkg/jwt/generator_test.go (1)
1-620: LGTM!Comprehensive test coverage across RSA/HMAC signing, access/identity tokens, error cases, and expiration handling. The test structure with subtests is clean and maintainable.
internal/domain/model/email.go (3)
8-13: LGTM!The field updates are appropriate:
omitemptyfor OTP and the rename toVerifiedaligns with the broader refactoring visible in related files.
24-38: LGTM!The EmailMessage struct design is clean and appropriate, with optional FromName and IsHTML fields for flexibility.
40-55: LGTM!The validation logic correctly checks required fields and validates email formats using the standard library. The optional From validation is a nice touch.
internal/infrastructure/authelia/sync_test.go (1)
59-84: LGTM!The mock implementations are appropriate for testing, with fixed return values (revision 1, OTP "123456") that are sufficient for test scenarios.
pkg/jwt/generator.go (1)
195-209: Claim construction looks correct.Type-specific claims (sub/scope for access; email (+ optional sub) for identity) are set clearly and in the right order.
- Refactored the SMTP client to use direct struct fields for host and port instead of nested config fields, improving clarity and consistency. - Updated the README to correct the path for the Email Verification Documentation, ensuring accurate navigation for users. - Minor adjustment in the NATS KV bucket configuration in the Helm chart for better readability. Jira Ticket: https://linuxfoundation.atlassian.net/browse/LFXV2-502 Reviewed with [GitHub Copilot](https://github.com/features/copilot) Signed-off-by: Mauricio Zanetti Salomao <[email protected]>
- Refactored the SendEmail method in the SMTP sender to access the host and port directly from the client struct, enhancing code clarity and consistency. Jira Ticket: https://linuxfoundation.atlassian.net/browse/LFXV2-502 Reviewed with [GitHub Copilot](https://github.com/features/copilot) Signed-off-by: Mauricio Zanetti Salomao <[email protected]>
- Revised comments in the Helm chart and SMTP client to clarify the purpose of the KV bucket and SMTP username. - Adjusted the user email retrieval logic to use the correct method for building the lookup key. - Improved documentation in the README regarding the test methods for identity tokens and corrected the function description for generating numeric strings. Jira Ticket: https://linuxfoundation.atlassian.net/browse/LFXV2-502 Reviewed with [GitHub Copilot](https://github.com/features/copilot) Signed-off-by: Mauricio Zanetti Salomao <[email protected]>
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (3)
internal/infrastructure/authelia/email.go (1)
49-51: Good: OTP no longer loggedThanks for removing OTP from logs; this avoids replay risks during the TTL window.
internal/service/message_handler.go (1)
389-391: Good: nil-guard added for userReader in LinkIdentityPrevents panic when orchestrator is built without a userReader.
internal/infrastructure/authelia/user.go (1)
310-350: Consume OTP after successful verification to prevent replayOTP remains valid until TTL expiry allowing reuse. Delete or mark used immediately after validation, before returning tokens.
otp, errGetVerificationCode := a.storage.GetVerificationCode(ctx, key) if errGetVerificationCode != nil { return nil, errGetVerificationCode } if otp != email.OTP { return nil, errs.NewValidation("invalid verification code") } + // Consume OTP to prevent replay within TTL + if err := a.storage.DeleteVerificationCode(ctx, key); err != nil { + return nil, errs.NewUnexpected("failed to consume verification code", err) + }Requires adding DeleteVerificationCode(ctx, key) to storage interfaces/impls.
🧹 Nitpick comments (10)
pkg/jwt/README.md (1)
30-30: Consider reducing exclamation marks for a more formal tone.The documentation contains 4 exclamation marks across key sections (warning, quick notes, and emphasis). For formal API documentation, these could be reduced to 1-2 strategic uses to maintain emphasis without appearing overly enthusiastic.
Example refactoring:
-**WARNING:** Default test methods use a singleton test key and are **only for testing**. Never use in production! +**WARNING:** Default test methods use a singleton test key and are **only for testing**—never use in production.-Perfect for unit tests - no need to manage keys! +Perfect for unit tests with no need to manage keys.-// Generate test token in one line! +// Generate test token in one line-**Default test methods are for testing only!** +**Default test methods are for testing only.**Also applies to: 177-177, 197-197, 397-397
pkg/constants/subjects.go (1)
37-39: Consider renaming for singular/plural consistency.The constant name
UserEmailReadSubjectuses singular "Email," but the subject value is"lfx.auth-service.user_emails.read"(plural "emails") and routes toGetUserEmails(plural). Since this endpoint retrieves multiple emails (primary + alternates), consider renaming toUserEmailsReadSubjectfor consistency.internal/infrastructure/smtp/client.go (2)
39-45: Enforce STARTTLS/TLS and add dial timeouts for SMTP reliability/securityCurrent path is opportunistic TLS and may hang without timeouts. Prefer smtp.Client with explicit STARTTLS (tls.Config{ServerName: c.Host}) and a net.Dialer with Timeout/Deadline; optionally support SMTPS (465). Add a require-TLS flag to fail fast if STARTTLS is unavailable.
56-75: Make SMTP defaults configurable and align “From” policyDefaults are fine for Mailpit, but production often needs:
- Configurable default port (587 preferred when auth is used).
- A policy to ensure the From matches the authenticated sender (some MTAs reject mismatches). Expose From default via config and document it in values/Helm.
internal/infrastructure/authelia/email.go (1)
36-41: Externalize From/Subject and include OTP TTL in the messageHard-coded "[email protected]" and generic subject can cause bounces or UX confusion. Make From/FromName/Subject configurable (env/values) and add the OTP validity window (e.g., “valid for 5 minutes”) to the body for clarity.
internal/service/message_handler.go (2)
93-99: Update stale comment: alternate email search is now supportedThe comment says alternate-email search is not available, but SearchUser implements it. Update to prevent confusion.
145-147: Log message wording“get user metadata” in getUserByInput is misleading since it’s reused by multiple endpoints. Rename log to “get user by input”.
internal/infrastructure/authelia/user.go (3)
365-377: Token parsing without signature verification (acceptable for mock; flag for future hardening)ParseUnverified is fine for this mock flow. For non-mock, set VerifySignature=true with the expected issuer/audience and public key.
420-444: Recheck cross-user uniqueness at link time (TOCTOU window)Between VerifyAlternateEmail and LinkIdentity, another account could link the same email. Add a final uniqueness check (by alternate_email index) just before UpdateUserWithRevision to fail fast or trigger merge logic.
279-307: SendVerificationAlternateEmail: ordering is good; consider idempotency and rate limitingEmail first, then persist OTP is acceptable. Consider:
- Idempotent resend (overwrite OTP) and rate-limit per email to reduce abuse.
- Include TTL in logs/metrics for observability.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
Disabled knowledge base sources:
- Jira integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (8)
charts/lfx-v2-auth-service/values.yaml(1 hunks)internal/infrastructure/authelia/email.go(1 hunks)internal/infrastructure/authelia/user.go(11 hunks)internal/infrastructure/smtp/client.go(1 hunks)internal/service/message_handler.go(7 hunks)pkg/constants/subjects.go(3 hunks)pkg/jwt/README.md(1 hunks)pkg/password/generate.go(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- pkg/password/generate.go
🧰 Additional context used
🧠 Learnings (5)
📓 Common learnings
Learnt from: mauriciozanettisalomao
PR: linuxfoundation/lfx-v2-auth-service#2
File: internal/infrastructure/auth0/user.go:16-21
Timestamp: 2025-09-19T17:04:25.227Z
Learning: In the LFX v2 Auth Service Auth0 implementation, mauriciozanettisalomao plans to redact sensitive user information (tokens, emails, metadata) from logs and use appropriate log levels during the actual Auth0 and Authelia implementation, rather than in the current stub/placeholder code.
📚 Learning: 2025-09-19T17:04:25.227Z
Learnt from: mauriciozanettisalomao
PR: linuxfoundation/lfx-v2-auth-service#2
File: internal/infrastructure/auth0/user.go:16-21
Timestamp: 2025-09-19T17:04:25.227Z
Learning: In the LFX v2 Auth Service Auth0 implementation, mauriciozanettisalomao plans to redact sensitive user information (tokens, emails, metadata) from logs and use appropriate log levels during the actual Auth0 and Authelia implementation, rather than in the current stub/placeholder code.
Applied to files:
internal/infrastructure/authelia/email.go
📚 Learning: 2025-09-22T14:40:52.872Z
Learnt from: mauriciozanettisalomao
PR: linuxfoundation/lfx-v2-auth-service#3
File: internal/infrastructure/auth0/user.go:157-163
Timestamp: 2025-09-22T14:40:52.872Z
Learning: In the LFX v2 Auth Service Auth0 implementation, mauriciozanettisalomao has chosen to defer implementing PII redaction for request body logging in the Auth0 Management API calls to upcoming PRs, acknowledging the security concern but prioritizing it for later implementation.
Applied to files:
internal/infrastructure/authelia/email.go
📚 Learning: 2025-09-22T14:40:04.765Z
Learnt from: mauriciozanettisalomao
PR: linuxfoundation/lfx-v2-auth-service#3
File: internal/infrastructure/auth0/user.go:201-209
Timestamp: 2025-09-22T14:40:04.765Z
Learning: In the LFX v2 Auth Service Auth0 implementation, mauriciozanettisalomao has chosen to defer implementing response body redaction for Auth0 Management API error logging to upcoming PRs, acknowledging the security concern but prioritizing it for later implementation.
Applied to files:
internal/infrastructure/authelia/email.go
📚 Learning: 2025-09-25T21:21:02.370Z
Learnt from: mauriciozanettisalomao
PR: linuxfoundation/lfx-v2-auth-service#5
File: pkg/httpclient/request.go:135-142
Timestamp: 2025-09-25T21:21:02.370Z
Learning: In the LFX v2 Auth Service Auth0 implementation, mauriciozanettisalomao has confirmed that Auth0 API error responses do not contain sensitive data, so full response body logging in error cases is acceptable without truncation or redaction.
Applied to files:
internal/infrastructure/authelia/email.go
🧬 Code graph analysis (4)
internal/infrastructure/smtp/client.go (2)
pkg/errors/server.go (1)
NewUnexpected(19-26)pkg/constants/global.go (4)
EmailSMTPHostEnvKey(71-71)EmailSMTPPortEnvKey(74-74)EmailSMTPUsernameEnvKey(83-83)EmailSMTPPasswordEnvKey(86-86)
internal/infrastructure/authelia/email.go (6)
internal/domain/port/email_sender.go (1)
EmailSender(13-16)pkg/password/generate.go (1)
OnlyNumbers(36-53)pkg/errors/server.go (1)
NewUnexpected(19-26)internal/domain/model/email.go (1)
EmailMessage(25-38)pkg/redaction/redaction.go (1)
RedactEmail(54-72)internal/infrastructure/smtp/sender.go (1)
NewSender(69-73)
internal/service/message_handler.go (7)
internal/domain/model/email.go (1)
internal/domain/port/transport_messenger.go (1)
TransportMessenger(7-11)internal/domain/model/user.go (1)
User(19-27)pkg/errors/server.go (1)
NewUnexpected(19-26)pkg/errors/client.go (1)
NewValidation(19-26)pkg/redaction/redaction.go (1)
Redact(22-42)internal/domain/model/identity.go (1)
LinkIdentity(7-21)
internal/infrastructure/authelia/user.go (12)
internal/infrastructure/authelia/models.go (1)
OIDCUserInfo(13-21)pkg/errors/client.go (1)
NewValidation(19-26)pkg/constants/user.go (1)
CriteriaTypeAlternateEmail(12-12)pkg/redaction/redaction.go (2)
RedactEmail(54-72)Redact(22-42)internal/domain/model/email.go (1)
pkg/errors/server.go (1)
NewUnexpected(19-26)internal/domain/model/user.go (1)
User(19-27)internal/domain/model/auth.go (1)
AuthResponse(8-14)pkg/jwt/generator.go (3)
GenerateSimpleTestIdentityTokenWithSubject(357-375)GenerateSimpleTestAccessToken(332-340)TokenType(48-48)pkg/jwt/parser.go (2)
ParseOptions(37-54)ParseUnverified(68-124)internal/domain/port/user.go (1)
UserReaderWriter(13-18)pkg/httpclient/config.go (1)
DefaultConfig(26-33)
🪛 LanguageTool
pkg/jwt/README.md
[style] ~397-~397: Using many exclamation marks might seem excessive (in this case: 4 exclamation marks for a text that’s 2692 characters long)
Context: ...efault test methods are for testing only!** - Keys are generated once and reused ...
(EN_EXCESSIVE_EXCLAMATION)
⏰ 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). (1)
- GitHub Check: MegaLinter
🔇 Additional comments (5)
pkg/jwt/README.md (2)
30-30: Markdown formatting issue resolved. ✓The emphasis markers are correctly formatted with no extra spaces — good catch implementing the previous feedback.
1-357: Comprehensive JWT documentation with clear examples. ✓The README is well-structured, covering quick-start, production, testing, use cases, error handling, and complete examples. Code snippets are functional and demonstrate best practices for both RSA and HMAC signing. Good separation between production and testing guidance.
charts/lfx-v2-auth-service/values.yaml (1)
45-65: ✓ Authelia email OTP KV bucket configuration is well-structured and aligns with PR objectives.The new
authelia_email_otp_kv_bucketblock (lines 45–65) is properly configured for the OTP flow:
- The corrected comment on line 52 (now reads "storing email OTP codes") resolves the previous review feedback.
ttl: 5mcorrectly implements the 5-minute OTP expiration window specified in PR objectives.history: 1is appropriate for ephemeral OTP data; only the current value is needed.- Sizing (
maxValueSize: 1024,maxBytes: 524288) is suitable for 6-digit OTP codes plus metadata.- Configuration mirrors the
authelia_users_kv_bucketpattern, ensuring consistency across KV bucket definitions.pkg/constants/subjects.go (1)
6-57: LGTM! File organization improvements and typo fix.The reorganization into logical groups (Lookup subjects, User read/write subjects, Email and Identity linking subjects) significantly improves readability. The typo from the previous review (
UserEmaiReadSubject) has been correctly fixed.internal/service/message_handler.go (1)
195-219: GetUserEmails response shape looks correctReturns primary_email and alternate_emails with redacted input on errors. LGTM.
- Updated the NewRegularWebAuthConfig function to return nil instead of an error when the AUTH0_REGULAR_WEB_CLIENT_ID is not set, with a TODO comment to implement the AUTH0 flow in the future. Jira Ticket: https://linuxfoundation.atlassian.net/browse/LFXV2-502 Signed-off-by: Mauricio Zanetti Salomao <[email protected]>
There was a problem hiding this comment.
Actionable comments posted: 1
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
Disabled knowledge base sources:
- Jira integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (1)
internal/infrastructure/auth0/token.go(1 hunks)
🧰 Additional context used
🧠 Learnings (1)
📓 Common learnings
Learnt from: mauriciozanettisalomao
PR: linuxfoundation/lfx-v2-auth-service#2
File: internal/infrastructure/auth0/user.go:16-21
Timestamp: 2025-09-19T17:04:25.227Z
Learning: In the LFX v2 Auth Service Auth0 implementation, mauriciozanettisalomao plans to redact sensitive user information (tokens, emails, metadata) from logs and use appropriate log levels during the actual Auth0 and Authelia implementation, rather than in the current stub/placeholder code.
⏰ 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). (1)
- GitHub Check: MegaLinter
- Adjusted the comment in the NewRegularWebAuthConfig function to improve readability by adding a space before the comment text. This change enhances code clarity without altering functionality. Jira Ticket: https://linuxfoundation.atlassian.net/browse/LFXV2-502 Signed-off-by: Mauricio Zanetti Salomao <[email protected]>
| Body: fmt.Sprintf("Your verification code is: %s", otp), | ||
| } | ||
|
|
||
| errSendEmail := a.emailSender.SendEmail(ctx, message) |
There was a problem hiding this comment.
I'm not sure if we need to apply a limit of 30 seconds or 1 minute to send the code. Not a blocker for this PR. Thanks
3 participants
Overview
Jira Ticket https://linuxfoundation.atlassian.net/browse/LFXV2-502
This PR implements alternate email linking functionality for the Authelia authentication provider, enabling users to verify and link additional email addresses to their accounts.
Key Features
Email Verification Flow:
SendVerificationAlternateEmail: Sends OTP verification code via SMTP to alternate emailVerifyAlternateEmail: Validates OTP and generates identity token for linkingIdentity Linking:
LinkIdentity: Links verified email identity to user accountTechnical Implementation:
NATS Subjects
lfx.auth-service.email_linking.send_verification- Initiates email verificationlfx.auth-service.email_linking.verify- Validates OTP and returns tokenlfx.auth-service.user_identity.link- Links verified identity to userThis brings Authelia to feature parity with Auth0 for alternate email linking workflows.
🧪 Email Linking Flow — Test Evidence
1️⃣ Initial state — existing alternate email
nats req --server nats://lfx-platform-nats.lfx.svc.cluster.local:4222 \ lfx.auth-service.user_emails.read superman | json_ppResponse:
{ "data": { "alternate_emails": [ { "email": "[email protected]", "verified": true } ], "primary_email": "[email protected]" }, "success": true }2️⃣ Request verification for new alternate email
Response:
{"success": true, "message": "alternate email verification sent"}3️⃣ Verify email (invalid attempt)
nats req --server nats://lfx-platform-nats.lfx.svc.cluster.local:4222 \ lfx.auth-service.email_linking.verify '{"email": "[email protected]", "otp": "111111"}'Response:
{"success": false, "error": "invalid verification code"}4️⃣ Verify email (valid attempt)
nats req --server nats://lfx-platform-nats.lfx.svc.cluster.local:4222 \ lfx.auth-service.email_linking.verify '{"email": "[email protected]", "otp": "648119"}'Response (truncated for brevity):
{ "success": true, "data": { "access_token": "<redacted>", "id_token": "<redacted>", "expires_in": 3600, "token_type": "Bearer" } }5️⃣ Link verified identity
nats req --server nats://lfx-platform-nats.lfx.svc.cluster.local:4222 \ lfx.auth-service.user_identity.link \ "{\"user\":{\"auth_token\":\"$AUTH_TOKEN\"},\"link_with\":{\"identity_token\":\"$ID_TOKEN\"}}"Response:
{"success": true, "message": "identity linked successfully"}6️⃣ Final state — new alternate email linked successfully
nats req --server nats://lfx-platform-nats.lfx.svc.cluster.local:4222 \ lfx.auth-service.user_emails.read superman | json_ppResponse:
{ "data": { "alternate_emails": [ { "email": "[email protected]", "verified": true }, { "email": "[email protected]", "verified": true } ], "primary_email": "[email protected]" }, "success": true }