Skip to content

docs: address auth-flows review, reorganize subject docs, and fix accuracy#54

Merged
fayazg merged 11 commits into
mainfrom
docs/LFXV2-888-update
Jun 18, 2026
Merged

docs: address auth-flows review, reorganize subject docs, and fix accuracy#54
fayazg merged 11 commits into
mainfrom
docs/LFXV2-888-update

Conversation

@fayazg

@fayazg fayazg commented Jun 5, 2026

Copy link
Copy Markdown
Contributor

Summary

Follow-up documentation cleanup after PR #20 (auth-flows docs) merged. Pure docs — no code changes.

  • Address @emsearcy's review on PR [LFXV2-888] Auth flows docs #20 (docs/auth-flows/):

    • access_token_mgmt_self scope corrected to update:current_user_metadata
      (was update:users* with an orphan asterisk).
    • Updated the M2M client description, the "M2M Client Scopes" security note, and
      the Flow A diagram to reflect that the Auth Service M2M client holds
      create/read/update/delete:users (Flow A still performs only reads).
  • Reorganize subject docs — moved username_lookups.md, alias.md,
    password_management.md, and impersonation.md into docs/subjects/ alongside
    the other NATS subject docs (via git mv, history preserved). Fixes previously
    broken sibling links from identity_linking.md and user_emails.md, and corrects
    relative paths to the internal/ READMEs.

  • Document user_emails.read input — the auth_token field accepts a verified
    JWT/Authelia token or a subject identifier (Auth0 sub with |, or Authelia UUID).
    Also corrected the top-level README (dropped the inaccurate "usernames" input and the
    stale "Authelia-only" note).

  • Docs-vs-code accuracy audit (full sweep of all docs against the handlers,
    providers, and pkg/constants):

    • email_verification.md: corrected error string to email already linked.
    • auth-flows: distinguished update:current_user_metadata (Flow C) from
      update:current_user_identities (Flow D, E); clarified Flow D's link
      payload/endpoint and Flow E's literal subjects; noted that auth0_mgmt/lfxv2
      are conceptual audience labels.
  • De-duplicate the README — replaced the per-subject lists in "Available
    Operations" with a link directory, so docs/ (and pkg/constants/subjects.go)
    remain the single source of truth and the README can't drift out of date.

Test plan

  • All relative links in docs/ resolve (full scan, 0 broken)
  • All README operation links resolve (11 docs)
  • No references to old root paths of moved files remain (repo-wide grep)
  • Audit findings verified against code (handlers, providers, pkg/constants);
    two agent false positives (indexer-contract, user_metadata username search)
    confirmed accurate and left unchanged
  • MegaLinter markdown checks (non-blocking) reviewed in CI

Refs LFXV2-888

🤖 Generated with Claude Code

fayazg and others added 5 commits June 4, 2026 17:39
Address review feedback from emsearcy on PR #20:

- access_token_mgmt_self uses update:current_user_metadata (self-service),
  not update:users; removed the orphan asterisk
  #20 (comment)

- Auth Service M2M client now holds create/read/update/delete:users; updated
  the client description, security consideration, and Flow A diagram
  #20 (comment)

Issue: LFXV2-888

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Signed-off-by: Fayaz G <5818912+fayazg@users.noreply.github.com>
Relocate username_lookups, alias, password_management, and impersonation
docs into docs/subjects/ alongside the other NATS subject docs. This fixes
the previously broken sibling links from identity_linking.md and
user_emails.md, and corrects relative paths to the internal/ READMEs.

Issue: LFXV2-888

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Signed-off-by: Fayaz G <5818912+fayazg@users.noreply.github.com>
Clarify that the user_emails.read auth_token field accepts either a verified
JWT/Authelia token or a subject identifier (Auth0 sub containing "|", or an
Authelia UUID) used directly via the service M2M token.

Issue: LFXV2-888

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Signed-off-by: Fayaz G <5818912+fayazg@users.noreply.github.com>
…note

Remove the inaccurate "usernames" input type and the stale "only supported
for Authelia" note; user_emails.read accepts a JWT token or a subject
identifier and is exercised on the Auth0 provider.

Issue: LFXV2-888

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Signed-off-by: Fayaz G <5818912+fayazg@users.noreply.github.com>
- email_verification: correct error string to "email already linked"
- auth-flows: distinguish update:current_user_metadata (C) from
  update:current_user_identities (D, E); clarify Flow D link
  payload/endpoint and Flow E subjects; note conceptual audience labels
- README: replace duplicated per-subject lists with a link directory so
  docs/ remain the single source of truth

Issue: LFXV2-888

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Signed-off-by: Fayaz G <5818912+fayazg@users.noreply.github.com>
Copilot AI review requested due to automatic review settings June 5, 2026 18:03
@fayazg fayazg requested a review from a team as a code owner June 5, 2026 18:03
@coderabbitai

coderabbitai Bot commented Jun 5, 2026

Copy link
Copy Markdown

Review Change Stack

Note

Reviews paused

It looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the reviews.auto_review.auto_pause_after_reviewed_commits setting.

Use the following commands to manage reviews:

  • @coderabbitai resume to resume automatic reviews.
  • @coderabbitai review to trigger a single review.

Use the checkboxes below for quick actions:

  • ▶️ Resume reviews
  • 🔍 Trigger review

Walkthrough

Refactors documentation: consolidates operations into a linked overview, clarifies auth-flow diagram tokens and NATS subjects, updates M2M scopes and token scope descriptions, standardizes email-verification errors, expands user_auth_token acceptance notes, and fixes relative cross-document links.

Changes

Documentation Clarification and Link Path Corrections

Layer / File(s) Summary
Operations Overview Restructure
README.md
Restructured "Available Operations" from detailed inline subject descriptions to a concise bulleted list linking to dedicated operation documentation pages and Auth Flows reference.
Auth Flows Token and Permission Clarifications
docs/auth-flows/README.md
M2M client now described with broader Management API scopes (create/read/update/delete), access_token_mgmt_self scope adjusted to update:current_user_metadata, and security statement clarified that M2M client has full scopes while Flow A executes read-only operations.
Flow Sequence Diagram Refinements
docs/auth-flows/A-auth-service-m2m-profile-lookup.md, docs/auth-flows/D-social-identity-linking.md, docs/auth-flows/E-passwordless-email-linking.md
Flow A token annotation simplified; Flow D NATS payload fields made explicit and Auth0 API call specified as POST /api/v2/users/{id}/identities using access_token_mgmt_self and id_token_social; Flow E NATS publish subjects changed to email_linking.send_verification and email_linking.verify.
Subject Documentation Token and Error Clarifications
docs/subjects/user_emails.md, docs/subjects/email_verification.md
user.auth_token now accepts either a validated JWT or a subject identifier used directly; NATS CLI examples added for both; email verification error examples standardized to "email already linked" with a note clarifying Mock provider uses "alternate email already linked".
Documentation Cross-Reference Link Path Corrections
docs/subjects/alias.md, docs/subjects/password_management.md, docs/subjects/username_lookups.md
Relative documentation paths corrected from ../internal/... to ../../internal/... to properly reference internal infrastructure files.

Estimated code review effort

🎯 1 (Trivial) | ⏱️ ~3 minutes

Possibly related PRs

  • linuxfoundation/lfx-v2-auth-service#20: Initial auth-flow documentation that this PR refines with permission updates, token scope clarifications, and NATS subject/endpoint detail improvements.
🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Title check ✅ Passed The title clearly summarizes the main changes: documentation updates addressing auth-flows review feedback, subject documentation reorganization, and accuracy corrections.
Description check ✅ Passed The description comprehensively details all changes including auth-flows corrections, file reorganization, documentation improvements, and accuracy audits across multiple files.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.

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

✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch docs/LFXV2-888-update

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

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Follow-up documentation cleanup after the auth-flows docs merge, aimed at improving accuracy and reducing drift by consolidating subject-operation references into the docs/ directory.

Changes:

  • Replaced the README’s per-subject operation lists with a directory of links into docs/subjects/.
  • Updated subject docs and auth-flow diagrams for corrected scopes, subjects, and error strings; fixed internal relative links after reorganizing subject docs.
  • Added/organized subject documentation pages (e.g., impersonation) and clarified request payload expectations in a few subject docs.

Reviewed changes

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

Show a summary per file
File Description
README.md Replaces expanded “Available Operations” content with a link directory to subject docs and auth-flows.
docs/subjects/username_lookups.md Updates relative link to internal Authelia README after moving into docs/subjects/.
docs/subjects/user_emails.md Clarifies user.auth_token accepted inputs and adds examples; updates relative link to internal Auth0 README.
docs/subjects/password_management.md Fixes internal Auth0 README relative links after moving into docs/subjects/.
docs/subjects/impersonation.md Adds documentation for the impersonation token exchange subject, payload, and behavior.
docs/subjects/email_verification.md Updates documented error string to match the service’s surfaced validation message.
docs/subjects/alias.md Fixes internal Auth0 README relative link after moving into docs/subjects/.
docs/auth-flows/README.md Updates token/scope descriptions and security notes to reflect current behavior and terminology.
docs/auth-flows/E-passwordless-email-linking.md Updates diagram steps to reference literal NATS subjects for email verification flow.
docs/auth-flows/D-social-identity-linking.md Updates diagram steps to reference literal NATS subjects and clarifies link payload details.
docs/auth-flows/A-auth-service-m2m-profile-lookup.md Adjusts diagram annotation around M2M token usage in Flow A.
Comments suppressed due to low confidence (1)

docs/subjects/username_lookups.md:53

  • The notes imply this operation “works with Auth0, Authelia, and mock repositories based on configuration” and that the returned value is the canonical user identifier. However, the handler for lfx.auth-service.username_to_sub is deterministic and always returns an Auth0-style sub (auth0|...) without consulting any configured provider, so this is not a canonical Authelia UUID in Authelia deployments.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment thread docs/subjects/user_emails.md Outdated
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
Signed-off-by: Fayaz G. <5818912+fayazg@users.noreply.github.com>
Copilot AI review requested due to automatic review settings June 5, 2026 18:09

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🧹 Nitpick comments (1)
docs/auth-flows/README.md (1)

48-49: 💤 Low value

Clarify the actual JWT audience values for reference.

The note explains that auth0_mgmt and lfxv2 are conceptual labels. While the Management API URL format is shown for auth0_mgmt, consider adding a concrete example for clarity.

📝 Suggested enhancement
-> **Note:** `auth0_mgmt` and `lfxv2` are conceptual audience labels used throughout these docs. The actual JWT `aud` claim is the Auth0 Management API URL (`https://{domain}/api/v2/`) for `auth0_mgmt`, and the configured LFX v2 API audience for `lfxv2`.
+> **Note:** `auth0_mgmt` and `lfxv2` are conceptual audience labels used throughout these docs. The actual JWT `aud` claim is the Auth0 Management API URL (e.g., `https://sso.linuxfoundation.org/api/v2/`) for `auth0_mgmt`, and the configured LFX v2 API audience (e.g., `https://api-gw.platform.linuxfoundation.org`) for `lfxv2`.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@docs/auth-flows/README.md` around lines 48 - 49, Update the note clarifying
the conceptual labels 'auth0_mgmt' and 'lfxv2' by adding concrete example JWT
aud values for each (e.g., for auth0_mgmt show a fully-formed Management API
audience like "https://your-auth0-domain.example/api/v2/" and for lfxv2 show a
sample configured API audience like "https://api.lfx.example/v2" or
"lfx-v2.example.com"), so readers can see exact formats to expect when
inspecting tokens; keep the original sentence but append these example audience
strings for clarity.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Nitpick comments:
In `@docs/auth-flows/README.md`:
- Around line 48-49: Update the note clarifying the conceptual labels
'auth0_mgmt' and 'lfxv2' by adding concrete example JWT aud values for each
(e.g., for auth0_mgmt show a fully-formed Management API audience like
"https://your-auth0-domain.example/api/v2/" and for lfxv2 show a sample
configured API audience like "https://api.lfx.example/v2" or
"lfx-v2.example.com"), so readers can see exact formats to expect when
inspecting tokens; keep the original sentence but append these example audience
strings for clarity.

ℹ️ Review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: 02b4ccaa-3ea1-43d9-ac09-50a9cf4f9593

📥 Commits

Reviewing files that changed from the base of the PR and between 114a756 and 3806d71.

📒 Files selected for processing (11)
  • README.md
  • docs/auth-flows/A-auth-service-m2m-profile-lookup.md
  • docs/auth-flows/D-social-identity-linking.md
  • docs/auth-flows/E-passwordless-email-linking.md
  • docs/auth-flows/README.md
  • docs/subjects/alias.md
  • docs/subjects/email_verification.md
  • docs/subjects/impersonation.md
  • docs/subjects/password_management.md
  • docs/subjects/user_emails.md
  • docs/subjects/username_lookups.md

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 10 out of 11 changed files in this pull request and generated 2 comments.

Comment thread README.md
Comment thread docs/subjects/user_emails.md
fayazg and others added 2 commits June 16, 2026 13:30
Address review comments from copilot-pull-request-reviewer:

- docs/subjects/user_emails.md: document authorization/trust constraints
  for the subject-identifier (no-token) mode of user.auth_token
- README.md: confirmed subject doc links now resolve under docs/subjects/
  (files were moved; no change needed)

Resolves 2 review threads.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Signed-off-by: Fayaz G <5818912+fayazg@users.noreply.github.com>
Copilot AI review requested due to automatic review settings June 16, 2026 19:55
@fayazg

fayazg commented Jun 16, 2026

Copy link
Copy Markdown
Contributor Author

Review Feedback Addressed

Commit: 408b5a3

Changes Made

  • docs/subjects/user_emails.md: Added an explicit Authorization note for the subject-identifier (no-token) form of user.auth_token — documents that it performs no token verification, is intended for trusted internal services only (the NATS bus is not exposed to end users), that the calling service must authorize the requesting principal, and that the JWT/Authelia token form should be used for end-user-initiated requests. (per copilot-pull-request-reviewer)

No Change Needed

  • README.md:60–72: The subject doc files were moved into docs/subjects/, so the README links now resolve correctly against the on-disk paths. Verified all targets exist. (flagged by copilot-pull-request-reviewer)

Threads Resolved

2 of 2 unresolved threads addressed in this iteration.

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 10 out of 11 changed files in this pull request and generated 2 comments.

Comment thread docs/subjects/email_verification.md
Comment thread docs/subjects/email_verification.md
Resolve docs/subjects/user_emails.md conflict: keep the detailed
set_primary preservation note from main (PR #56) over the prior
one-line wording. The README link depth (../../) already matched on
both sides; PR #54's auth_token subject-identifier docs are retained.

Signed-off-by: Fayaz G <5818912+fayazg@users.noreply.github.com>
@fayazg

fayazg commented Jun 16, 2026

Copy link
Copy Markdown
Contributor Author

Merged main to resolve conflict (e9bb85c)

main advanced (PR #56preserve old primary email before switching) and edited docs/subjects/user_emails.md, which conflicted with this branch's reorganization of the same file.

Resolution — only docs/subjects/user_emails.md conflicted, in one bullet under the set_primary Important Notes:

  • Kept main's detailed primary-preservation note (the accurate current behavior: any email/OTP identity or a verified Google login is sufficient; an unverified Google identity, a non-Google provider, or no backing identity is preserved as a verified, user-removable email identity; fail-loud if preservation fails) over this branch's prior one-line wording.
  • The Auth0 README link depth (../../) was identical on both sides.
  • This branch's substantive additions are retained: the user.auth_token subject-identifier docs + authorization warning, and the subject-identifier CLI example.

The merge also brought main's code onto this branch (PR #56 adapter changes + PR #53 NATS tracing). go build ./... and go test ./... pass locally; no further doc edits were needed.

Address review comments from copilot-pull-request-reviewer:

- docs/subjects/email_verification.md: the conflict error was documented
  as the fixed string "email already linked", but it is provider-specific
  (the mock provider returns "alternate email already linked"). Add a note
  under both "Email Already Linked" error replies that the error text is a
  human-readable diagnostic, not a stable contract, and consumers should
  branch on the success flag; note the mock variant in the Mock provider
  section.

Resolves 2 review threads.

Signed-off-by: Fayaz G <5818912+fayazg@users.noreply.github.com>
Copilot AI review requested due to automatic review settings June 16, 2026 21:49
@fayazg

fayazg commented Jun 16, 2026

Copy link
Copy Markdown
Contributor Author

Review Feedback Addressed

Commit: bb01a03

Changes Made

  • docs/subjects/email_verification.md: the conflict error was documented as the fixed string email already linked, but that's provider-specific — the orchestrator pre-check returns email already linked (accurate for Auth0), while the mock provider returns alternate email already linked for the same condition. Added a note under both "Email Already Linked" error replies that the error text is a human-readable diagnostic (not a stable contract) and consumers should branch on the success: false flag; also noted the mock's variant in the Mock provider-notes section. (per copilot[bot] — two threads, same point)

Threads Resolved

2 of 2 review threads addressed and resolved.

Chose a doc-only clarification over normalizing the mock's error string — the mock's more specific message is useful, and changing a test double's behavior is out of scope for a docs PR.

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

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

Comment thread docs/auth-flows/README.md Outdated
Address review comment from copilot-pull-request-reviewer:

- docs/auth-flows/README.md: the Token Overview listed access_token_m2m_read
  with only read:users, contradicting the client description and Security
  Considerations (which grant create/read/update/delete:users). The M2M token
  is requested without a scope param, so it carries all granted scopes; update
  the Scope cell to list them and note Flow A uses read:users, matching the
  Flow A diagram.

Resolves 1 review thread.

Signed-off-by: Fayaz G <5818912+fayazg@users.noreply.github.com>
@fayazg

fayazg commented Jun 16, 2026

Copy link
Copy Markdown
Contributor Author

Review Feedback Addressed

Commit: bd1f784

Changes Made

  • docs/auth-flows/README.md: the Token Overview row for access_token_m2m_read listed only read:users, contradicting the client description and Security Considerations (which grant create/read/update/delete:users). Since the M2M token is requested without a scope param, it carries all granted scopes — updated the Scope cell to list them and note Flow A uses read:users, matching the rest of the doc and the Flow A diagram. (per copilot[bot])

Threads Resolved

1 of 1 review thread addressed and resolved.

@fayazg fayazg merged commit 79eb8c7 into main Jun 18, 2026
10 checks passed
@fayazg fayazg deleted the docs/LFXV2-888-update branch June 18, 2026 16:13
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants