Skip to content

docs: Web Auth documentation#1235

Merged
Assem-Uber merged 6 commits intocadence-workflow:masterfrom
Assem-Uber:feature/auth-docs
Apr 10, 2026
Merged

docs: Web Auth documentation#1235
Assem-Uber merged 6 commits intocadence-workflow:masterfrom
Assem-Uber:feature/auth-docs

Conversation

@Assem-Uber
Copy link
Copy Markdown
Contributor

Summary

Create web auth document that includes:

Assem-Uber and others added 5 commits April 10, 2026 10:53
Co-authored-by: Adhitya Mamallan <adi1998is@gmail.com>
Co-authored-by: Adhitya Mamallan <adi1998is@gmail.com>
Co-authored-by: Adhitya Mamallan <adi1998is@gmail.com>
participant W as Cadence Web
participant C as Cadence server

B->>W: Set cadence-authorization cookie
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

💡 Quality: Sequence diagram step 1 description is inaccurate

Step 1 says B->>W: Set cadence-authorization cookie — but the browser doesn't "set" a cookie on the server. The previous wording ("Request with cadence-authorization cookie") was more accurate: the browser sends a request that includes the cookie. The removed W->>W: resolveAuthContext line was also helpful for showing what the server does on receipt.

Also, the note on line 95 has a typo: "dependant" → "dependent".

Suggested fix:

  B->>W: Request with cadence-authorization cookie

...

Note: 4-5 are not dependent on 2-3 ...

Was this helpful? React with 👍 / 👎 | Reply gitar fix to apply this suggestion

@gitar-bot
Copy link
Copy Markdown

gitar-bot bot commented Apr 10, 2026

Code Review 👍 Approved with suggestions 2 resolved / 3 findings

Web Auth documentation adds clear sequence diagrams and configuration guidance for authentication flows, with two issues resolved: removed private key from repository and cleaned up jwt.js debug code. Consider correcting step 1 description in the sequence diagram to accurately reflect browser cookie behavior.

💡 Quality: Sequence diagram step 1 description is inaccurate

📄 docs/auth.md:87 📄 docs/auth.md:95

Step 1 says B->>W: Set cadence-authorization cookie — but the browser doesn't "set" a cookie on the server. The previous wording ("Request with cadence-authorization cookie") was more accurate: the browser sends a request that includes the cookie. The removed W->>W: resolveAuthContext line was also helpful for showing what the server does on receipt.

Also, the note on line 95 has a typo: "dependant" → "dependent".

Suggested fix
  B->>W: Request with cadence-authorization cookie

...

Note: 4-5 are not dependent on 2-3 ...
✅ 2 resolved
Security: Private key committed to repository

📄 scripts/keytest:1-15 📄 scripts/keytest.pub:1-9 📄 scripts/jwt.js:7 📄 docker-compose-backend-services-auth.yml:38-39
The file scripts/keytest contains an RSA private key in plaintext, and scripts/jwt.js reads it to sign JWTs. Even if this is intended only for local development/testing, committing private keys to a repository is a security anti-pattern:

  1. The key is now permanently in git history even if later deleted.
  2. If this key (or the corresponding public key in scripts/keytest.pub) is used in any non-local environment, tokens can be forged by anyone with repo access.
  3. The docker-compose file (docker-compose-backend-services-auth.yml, line 39) configures the Cadence server with OAUTH_PUBLIC_KEY=config/credentials/keytest.pub and OAUTH_PRIVATE_KEY=config/credentials/keytest, suggesting these keys may be mounted into running services.

Recommend:

  • Add scripts/keytest and scripts/keytest.pub to .gitignore.
  • Provide a key-generation script (e.g. openssl genrsa) instead of checking in actual key material.
  • Document in the README that developers should generate their own key pair for local auth testing.
Quality: jwt.js has commented-out debug code and no trailing newline

📄 scripts/jwt.js:11 📄 scripts/jwt.js:27-28
scripts/jwt.js contains commented-out lines (// const exp = now + 30; on line 11 and // console.log(...) on line 28) that look like leftover debugging. The file also lacks a trailing newline. These should be cleaned up before merging to keep the developer tooling tidy.

🤖 Prompt for agents
Code Review: Web Auth documentation adds clear sequence diagrams and configuration guidance for authentication flows, with two issues resolved: removed private key from repository and cleaned up jwt.js debug code. Consider correcting step 1 description in the sequence diagram to accurately reflect browser cookie behavior.

1. 💡 Quality: Sequence diagram step 1 description is inaccurate
   Files: docs/auth.md:87, docs/auth.md:95

   Step 1 says `B->>W: Set cadence-authorization cookie` — but the browser doesn't "set" a cookie on the server. The previous wording ("Request with cadence-authorization cookie") was more accurate: the browser *sends* a request that includes the cookie. The removed `W->>W: resolveAuthContext` line was also helpful for showing what the server does on receipt.
   
   Also, the note on line 95 has a typo: "dependant" → "dependent".

   Suggested fix:
     B->>W: Request with cadence-authorization cookie
   
   ...
   
   Note: 4-5 are not dependent on 2-3 ...

Options

Auto-apply is off → Gitar will not commit updates to this branch.
Display: compact → Showing less information.

Comment with these commands to change:

Auto-apply Compact
gitar auto-apply:on         
gitar display:verbose         

Was this helpful? React with 👍 / 👎 | Gitar

@Assem-Uber Assem-Uber merged commit aef363b into cadence-workflow:master Apr 10, 2026
4 checks passed
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.

2 participants