Implement memory session hooks and enhance Management API documentation - #28
Conversation
- Introduce a new `memorySessionHooks` struct to manage session state in memory. - Add methods for setting and getting post-auth redirect, state, and raw token with appropriate error handling. - Update `NewClientCredentialsFlow` to initialize session hooks with memory storage. - Include comprehensive unit tests to validate the functionality and thread safety of the memory session hooks.
- Introduce a new README_MANAGEMENT_API.md file detailing the Kinde Management API, including authentication, session management, and usage examples. - Update the main README.md to link to the new Management API documentation for better accessibility and guidance on using the API. - Enhance existing sections with references to the new documentation for clarity on Management API usage.
WalkthroughAdds a concurrency-safe in-memory SessionHooks and defaults NewClientCredentialsFlow to it when nil; adds Management API documentation and README restructuring; extends the authorization_code flow with InjectTokenMiddleware and TokenFromContext plus examples and tests; adds jwt README update and related tests. Changes
Sequence Diagram(s)sequenceDiagram
participant App
participant CC as ClientCredentialsFlow
participant Mem as MemorySessionHooks
Note over CC,Mem #cfe8ff: Default init when no session hooks provided
App->>CC: NewClientCredentialsFlow(opts with sessionHooks=nil)
CC->>Mem: NewMemorySessionHooks()
CC-->>App: flow (sessionHooks -> MemorySessionHooks)
sequenceDiagram
participant Client
participant MW as InjectTokenMiddleware
participant AC as AuthorizationCodeFlow
participant Handler
Note over MW,AC #f0f9e8: Request handling with token injection
Client->>MW: HTTP request
MW->>AC: GetToken(ctx)
alt token available
AC-->>MW: jwt.Token
MW->>Handler: request with token in context
else token missing/error
MW->>Handler: original request (no token)
end
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related PRs
Suggested reviewers
Tip 🔌 Remote MCP (Model Context Protocol) integration is now available!Pro plan users can now connect to remote MCP servers from the Integrations page. Connect with popular remote MCPs such as Notion and Linear to add more context to your reviews and chats. ✨ Finishing Touches🧪 Generate unit tests
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. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. CodeRabbit Commands (Invoked using PR/Issue comments)Type Other keywords and placeholders
CodeRabbit Configuration File (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 4
🧹 Nitpick comments (18)
README.md (3)
99-100: Remove angle brackets around the Management API domain in code exampleAngle brackets become part of the string literal in Go. Use a plain URL to avoid invalid audience configuration.
- client_credentials.WithKindeManagementAPI("<https://my_kinde_tenant.kinde.com>"), // adds kinde management API audience - see README_MANAGEMENT_API.md for details + client_credentials.WithKindeManagementAPI("https://my_kinde_tenant.kinde.com"), // adds Kinde Management API audience - see README_MANAGEMENT_API.md for details
131-136: Clarify prerequisites and audience for Management API in this sectionConsider explicitly stating here that:
- WithKindeManagementAPI("https://.kinde.com") must be set (or an equivalent audience must be provided).
- Required scopes must be configured on the M2M app (link to the scopes list in README_MANAGEMENT_API.md).
This keeps the short section actionable without forcing a context switch.
164-172: Examples index is helpful; consider linking to runnable foldersIf examples/cli and examples/gin-chat are runnable, linking them here improves DX.
oauth2/client_credentials/client_credentials.go (3)
64-66: Defaulting to in-memory session hooks: good DX, add a safety signal for prodGreat DX win to avoid nil errors. However, silently defaulting to memory can surprise production users who intended persistent/secure storage. Two low-friction options:
- Log a one-time warning when memory session is used.
- Or add an option like WithDefaultMemorySession() and keep the current behavior, but print a warning only if offline/refresh tokens are requested.
If you prefer minimal change, a package-level doc comment in NewClientCredentialsFlow warning about the default is sufficient.
Would you like a small patch that injects an optional logger and prints a warning when memory storage is chosen?
15-16: Remove unused TokenTypeTokenType is declared but unused in this file. Safe to remove to reduce noise.
-type ( - TokenType string +type (
31-35: Name style: JWKS_URL is non-idiomatic for exported Go identifiersGo style favors JWKSURL (no underscore). Renaming would be a breaking change; consider deferring or introducing a getter method (JWKSURL()) while deprecating the field in a future minor release.
README_MANAGEMENT_API.md (2)
89-96: Remove angle brackets in default memory session exampleBrackets become part of the string. Use a plain URL.
- client_credentials.WithKindeManagementAPI("<https://my_kinde_tenant.kinde.com>"), + client_credentials.WithKindeManagementAPI("https://my_kinde_tenant.kinde.com"),
191-201: Scope the operations list or link to authoritative docsThese operations evolve. Consider linking to the upstream API reference to avoid drift, and add a note that SDK types are generated from the latest spec.
oauth2/client_credentials/memorySession_test.go (4)
165-207: Concurrency test is good; consider running tests with -race in CIThe pattern exercises concurrent Set/Get. To maximize detection, ensure CI runs
go test -race ./.... Also consider adding t.Parallel() to top-level tests that don’t share state to reduce wall time.Would you like me to open a small CI patch enabling
-race?
239-269: Type-safety test reaches into internals; acceptable trade-off but brittleThis validates resilience to type corruption by directly mutating unexported fields. If we later change the backing store, this will break. Optional: gate these checks behind build tag
//go:build !pureor refactor to use exported behavior only. Keeping as-is is fine for now.
142-163: Consider asserting via sentinel errors instead of string matchingString comparisons to error messages are brittle. If we introduce package-level sentinel errors (e.g., var ErrTokenNotFound = errors.New("...")), tests can use errors.Is for stability. This requires small changes to memorySession.go.
1-270: Add a test for default memory session injection in NewClientCredentialsFlowSince the flow now defaults to memory hooks when none are provided, add a unit test to lock this behavior in.
New file suggestion: oauth2/client_credentials/client_credentials_default_session_test.go
+package client_credentials + +import ( + "testing" +) + +func TestNewClientCredentialsFlow_DefaultsMemorySession(t *testing.T) { + flowIface, err := NewClientCredentialsFlow("https://tenant.kinde.com", "id", "secret") + if err != nil { + t.Fatalf("unexpected error: %v", err) + } + flow, ok := flowIface.(*ClientCredentialsFlow) + if !ok { + t.Fatalf("unexpected flow type: %T", flowIface) + } + if flow.sessionHooks == nil { + t.Fatalf("expected non-nil sessionHooks by default") + } +}oauth2/client_credentials/memorySession.go (6)
62-76: *Return and store a copy of oauth2.Token to avoid external mutationReturning the internal pointer allows callers to mutate shared state. Store and return a shallow copy to reduce accidental data races and unintended side effects.
-func (t *memorySessionHooks) GetRawToken() (*oauth2.Token, error) { +func (t *memorySessionHooks) GetRawToken() (*oauth2.Token, error) { t.mu.RLock() defer t.mu.RUnlock() - val, exists := t.sessionState["kinde_token"] + val, exists := t.sessionState[keyKindeToken] if !exists || val == nil { return nil, fmt.Errorf("kinde_token not found in session state") } - token, ok := val.(*oauth2.Token) + token, ok := val.(*oauth2.Token) if !ok { return nil, fmt.Errorf("kinde_token is not of type *oauth2.Token") } - return token, nil + // return a shallow copy + tokCopy := new(oauth2.Token) + *tokCopy = *token + return tokCopy, nil }-func (t *memorySessionHooks) SetRawToken(token *oauth2.Token) error { +func (t *memorySessionHooks) SetRawToken(token *oauth2.Token) error { if token == nil { return fmt.Errorf("token cannot be nil") } t.mu.Lock() defer t.mu.Unlock() - t.sessionState["kinde_token"] = token + tokCopy := new(oauth2.Token) + *tokCopy = *token + t.sessionState[keyKindeToken] = tokCopy return nil }Note: Test equality uses testify/assert.Equal, which deep-compares struct values; it should continue to pass with copies.
10-19: Extract map keys into typed constants to prevent typosUsing raw strings risks drift across files/tests. Introduce constants and replace usages.
+const ( + keyPostAuthRedirect = "post_auth_redirect" + keyState = "state" + keyKindeToken = "kinde_token" +)Then replace "post_auth_redirect", "state", and "kinde_token" occurrences with the constants.
21-45: Nit: comment typos (“implements SessionHooks”) and namingThe interface in this package is ISessionHooks; adjust comments for accuracy and align method names.
-// GetPostAuthRedirect implements SessionHooks. +// GetPostAuthRedirect implements ISessionHooks (shared behavior across flows). ... -// SetPostAuthRedirect implements SessionHooks. +// SetPostAuthRedirect implements ISessionHooks (shared behavior across flows).
47-60: Same comment fix for state gettersUpdate comments to refer to ISessionHooks.
62-66: Comment mismatch: method is GetRawToken, comment says GetTokenAlign the comment with the method name.
-// GetToken implements SessionHooks. +// GetRawToken implements ISessionHooks.
88-97: Comment mismatch: method is SetRawToken, comment says SetTokenAlign the comment with the method name.
-// SetToken implements SessionHooks. +// SetRawToken implements ISessionHooks.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (5)
README.md(4 hunks)README_MANAGEMENT_API.md(1 hunks)oauth2/client_credentials/client_credentials.go(1 hunks)oauth2/client_credentials/memorySession.go(1 hunks)oauth2/client_credentials/memorySession_test.go(1 hunks)
🧰 Additional context used
🧬 Code graph analysis (3)
oauth2/client_credentials/client_credentials.go (1)
oauth2/client_credentials/memorySession.go (1)
NewMemorySessionHooks(15-19)
oauth2/client_credentials/memorySession_test.go (1)
oauth2/client_credentials/memorySession.go (1)
NewMemorySessionHooks(15-19)
oauth2/client_credentials/memorySession.go (1)
jwt/jwt.go (1)
Token(22-27)
🪛 LanguageTool
README_MANAGEMENT_API.md
[grammar] ~11-~11: There might be a mistake here.
Context: ...uisites - A Kinde account with a tenant - A Machine-to-Machine (M2M) application c...
(QB_NEW_EN)
[grammar] ~12-~12: There might be a mistake here.
Context: ...lication configured in your Kinde tenant - Management API access enabled for your M...
(QB_NEW_EN)
[grammar] ~13-~13: There might be a mistake here.
Context: ... access enabled for your M2M application - Appropriate scopes configured for the op...
(QB_NEW_EN)
[grammar] ~79-~79: There might be a mistake here.
Context: ...ger on Windows, Secret Service on Linux) - Automatically handles token chunking for...
(QB_NEW_EN)
[grammar] ~80-~80: There might be a mistake here.
Context: ... handles token chunking for large tokens - Secure by default with proper access con...
(QB_NEW_EN)
[grammar] ~81-~81: There might be a mistake here.
Context: ...e by default with proper access controls - Ideal for command-line tools and desktop...
(QB_NEW_EN)
[grammar] ~100-~100: There might be a mistake here.
Context: ...in memory (lost when process terminates) - Thread-safe with proper locking - Good f...
(QB_NEW_EN)
[grammar] ~101-~101: There might be a mistake here.
Context: ...nates) - Thread-safe with proper locking - Good for testing and short-lived process...
(QB_NEW_EN)
[grammar] ~102-~102: There might be a mistake here.
Context: ...od for testing and short-lived processes - No external dependencies ### Custom Ses...
(QB_NEW_EN)
[grammar] ~111-~111: There might be a mistake here.
Context: ...base-backed storage for web applications - Redis for distributed systems - Encrypte...
(QB_NEW_EN)
[grammar] ~112-~112: There might be a mistake here.
Context: ...ications - Redis for distributed systems - Encrypted file storage - Integration wit...
(QB_NEW_EN)
[grammar] ~113-~113: There might be a mistake here.
Context: ...ributed systems - Encrypted file storage - Integration with existing session manage...
(QB_NEW_EN)
[grammar] ~218-~218: There might be a mistake here.
Context: ... Bad Request**: Invalid input parameters - 401 Unauthorized: Invalid or expired t...
(QB_NEW_EN)
[grammar] ~219-~219: There might be a mistake here.
Context: ...Unauthorized**: Invalid or expired token - 403 Forbidden: Insufficient permission...
(QB_NEW_EN)
[grammar] ~220-~220: There might be a mistake here.
Context: ...nsufficient permissions or missing scope - 429 Too Many Requests: Rate limiting a...
(QB_NEW_EN)
[grammar] ~221-~221: There might be a mistake here.
Context: ...o Many Requests**: Rate limiting applied - 500 Internal Server Error: Server-side...
(QB_NEW_EN)
README.md
[grammar] ~5-~5: There might be a mistake here.
Context: ...EMENT_API.md](README_MANAGEMENT_API.md). 
(QB_NEW_EN)
🔇 Additional comments (3)
README.md (1)
5-5: Great addition: prominent link to Management API guideThe top-level pointer to README_MANAGEMENT_API.md is helpful and discoverable. No issues.
README_MANAGEMENT_API.md (1)
136-143: Importmanagement_apiin README exampleThe usage snippet in
README_MANAGEMENT_API.md(around lines 136–143) referencesmanagement_api.CreateApplicationReqandmanagement_api.CreateApplicationReqTypeRegbut only imports the corekindepackage. Add the missing import and confirm the enum name:import ( "github.com/kinde-oss/kinde-go/kinde" + "github.com/kinde-oss/kinde-go/kinde/management_api" )The enum value
CreateApplicationReqTypeRegis defined in the generated SDK (kinde/management_api/oas_schemas_gen.go:1057) and matches the example, so no change to the identifier is needed.oauth2/client_credentials/memorySession_test.go (1)
10-16: LGTM: constructor test is clear and targetedInitialization checks cover nil and empty state.
…ith improved descriptions and examples
There was a problem hiding this comment.
Actionable comments posted: 7
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
README.md (1)
65-69: Fix typo and code fence opening“Client willl” has an extra “l”. Also, the code block opens with four backticks; stick to triple backticks for consistency.
Apply this diff:
-Client willl manage tokens in the background, reading/persisting them to provided the session storage. +Client will manage tokens in the background, reading/persisting them to the provided session storage. - -````go +```go
♻️ Duplicate comments (1)
README_MANAGEMENT_API.md (1)
121-134: Make NewManagementAPI example copy-pastable (imports, ctx, and flow var)This snippet references
ctxand “” without declarations, making it non-runnable. Prior review already flagged this; reiterating with a concrete patch.Apply this diff:
import ( - "github.com/kinde-oss/kinde-go/kinde" + "context" + "github.com/kinde-oss/kinde-go/kinde" ) -managementApi, err := kinde.NewManagementAPI(ctx, "<kinde domain>", <client credentials flow>) +ctx := context.Background() +managementAPI, err := kinde.NewManagementAPI(ctx, "https://my_kinde_tenant.kinde.com", kindeClient) if err != nil { // Handle error }
🧹 Nitpick comments (8)
README_MANAGEMENT_API.md (3)
45-50: Clarify interaction between WithKindeManagementAPI and WithAudienceWhen
WithKindeManagementAPI(...)is used, it already adds the Management API audience. CallingWithAudience(...)with the same or different value can be confusing. Add a note thatWithAudienceis typically unnecessary for Management API-only access, and if both are supplied, the combined audiences must be valid.Proposed wording tweak:
- **`WithKindeManagementAPI()`**: This is essential for Management API access. It automatically adds the correct Management API audience to your token requests. + **`WithKindeManagementAPI()`**: Essential for Management API access. It automatically adds the Management API audience to token requests. If you also pass `WithAudience(...)`, ensure the combined audiences match your API requirements.
60-76: Unify URL formatting and remove angle brackets around literalsAngle brackets render inside code and confuse copy-paste. In later sections you still use
<https://...>.Apply this diff to keep plain URL literals:
- client_credentials.WithKindeManagementAPI("<https://my_kinde_tenant.kinde.com>"), + client_credentials.WithKindeManagementAPI("https://my_kinde_tenant.kinde.com"),
180-194: Minor: reinforce error handling styleThe HTTP example correctly handles
GetClienterrors. Consider showing a non-fatal handling path (return the error to the caller) instead oflog.Fatalfin earlier snippet for consistency across docs. Optional.Alternative:
client, err := kindeClient.GetClient(context.Background()) if err != nil { return fmt.Errorf("init client: %w", err) }README.md (3)
39-53: Tighten typos and URL formatting in the Client Credentials snippetFix small typos and remove angle brackets around the Management API URL.
Apply this diff:
- client_credentials.WithAudience("[your API audience]"), // optioanlly include your API audience + client_credentials.WithAudience("[your API audience]"), // optionally include your API audience client_credentials.WithScopes() // optional - request API scopes - client_credentials.WithKindeManagementAPI("<https://my_kinde_tenant.kinde.com>"), // adds kinde management API audience - see README_MANAGEMENT_API.md for details + client_credentials.WithKindeManagementAPI("https://my_kinde_tenant.kinde.com"), // adds Kinde Management API audience - see README_MANAGEMENT_API.md for details client_credentials.WithSessionHooks(<ISessionHooks implementation>), // example of CLI is cli.NewCliSession(...) - client_credentials.WithTokenValidation( // validates tokens when a new token is aquired + client_credentials.WithTokenValidation( // validates tokens when a new token is acquired true, // will validate token signature via JWKS jwt.WillValidateAlgorithm(), // will validate the token alg is RS256 jwt.WillValidateAudience("<your API audience>"), // will confirm that received token includes correct audience ), )
35-38: Mention default memory session to match new behaviorDocs say “Please implement session hooks,” but the flow now initializes a default in-memory session when none is provided. Add a clarifying sentence.
Proposed tweak:
-This flow is designed for machine-to-machine communication which doesn't involve human input. It requires Kinde M2M application. Please implement session hooks to store tokens accordingly to your security practices. +This flow is designed for machine-to-machine communication which doesn't involve human input. It requires a Kinde M2M application. If you don't provide session hooks, the SDK uses a thread-safe in-memory session by default; for production, implement session hooks that meet your security practices.
148-151: Fix link text: remove placeholder “[link-to-kinde-doc]” wrapperThe nested brackets render oddly.
Apply this diff:
-For details on integrating this SDK into your project, head over to the [Kinde docs](https://kinde.com/docs/) and see the [Go SDK](<[link-to-kinde-doc](https://kinde.com/docs/developer-tools/)>) doc 👍🏼. +For details on integrating this SDK into your project, head over to the [Kinde docs](https://kinde.com/docs/) and the [Go SDK docs](https://kinde.com/docs/developer-tools/) 👍🏼.oauth2/authorization_code/README.md (2)
79-83: Specify language on fenced code blockMissing language triggers MD040 and hurts syntax highlighting.
Apply this diff:
-``` +```go // Set up your callback route http.HandleFunc("/callback", func(w http.ResponseWriter, r *http.Request) { kindeAuthFlow.AuthorizationCodeReceivedHandler(w, r) })
242-246: Clarify dependencies: oauth2 is not part of the Go standard libraryList
golang.org/x/oauth2explicitly and avoid implying it’s stdlib.Apply this diff:
-This package requires Go 1.24+ and depends on the following packages: +This package requires Go 1.24+ and depends on: -- `github.com/kinde-oss/kinde-go/jwt` - For JWT token handling and validation -- Standard Go packages: `context`, `net/http`, `oauth2` +- `github.com/kinde-oss/kinde-go/jwt` — JWT handling and validation +- `golang.org/x/oauth2` — OAuth 2.0 types and helpers +- Standard library packages: `context`, `net/http`
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (3)
README.md(5 hunks)README_MANAGEMENT_API.md(1 hunks)oauth2/authorization_code/README.md(1 hunks)
🧰 Additional context used
🪛 LanguageTool
README.md
[grammar] ~5-~5: There might be a mistake here.
Context: ...EMENT_API.md](README_MANAGEMENT_API.md).
(QB_NEW_EN)
[grammar] ~26-~26: There might be a mistake here.
Context: ...orization code flow for web applications - Device authorization flow for devices wi...
(QB_NEW_EN)
[grammar] ~27-~27: There might be a mistake here.
Context: ... devices with limited input capabilities - Session management and token validation ...
(QB_NEW_EN)
[grammar] ~28-~28: There might be a mistake here.
Context: ... Session management and token validation - Offline support with refresh token manag...
(QB_NEW_EN)
README_MANAGEMENT_API.md
[grammar] ~11-~11: There might be a mistake here.
Context: ...uisites - A Kinde account with a tenant - A Machine-to-Machine (M2M) application c...
(QB_NEW_EN)
[grammar] ~12-~12: There might be a mistake here.
Context: ...lication configured in your Kinde tenant - Management API access enabled for your M...
(QB_NEW_EN)
[grammar] ~13-~13: There might be a mistake here.
Context: ... access enabled for your M2M application - Appropriate scopes configured for the op...
(QB_NEW_EN)
[grammar] ~105-~105: There might be a mistake here.
Context: ...in memory (lost when process terminates) - Thread-safe with proper locking - Good f...
(QB_NEW_EN)
[grammar] ~106-~106: There might be a mistake here.
Context: ...nates) - Thread-safe with proper locking - Good for testing and short-lived process...
(QB_NEW_EN)
[grammar] ~107-~107: There might be a mistake here.
Context: ...od for testing and short-lived processes - No external dependencies ### Custom Ses...
(QB_NEW_EN)
[grammar] ~116-~116: There might be a mistake here.
Context: ...base-backed storage for web applications - Redis for distributed systems - Encrypte...
(QB_NEW_EN)
[grammar] ~117-~117: There might be a mistake here.
Context: ...ications - Redis for distributed systems - Encrypted file storage - Integration wit...
(QB_NEW_EN)
[grammar] ~118-~118: There might be a mistake here.
Context: ...ributed systems - Encrypted file storage - Integration with existing session manage...
(QB_NEW_EN)
[grammar] ~223-~223: There might be a mistake here.
Context: ... Bad Request**: Invalid input parameters - 401 Unauthorized: Invalid or expired t...
(QB_NEW_EN)
[grammar] ~224-~224: There might be a mistake here.
Context: ...Unauthorized**: Invalid or expired token - 403 Forbidden: Insufficient permission...
(QB_NEW_EN)
[grammar] ~225-~225: There might be a mistake here.
Context: ...nsufficient permissions or missing scope - 429 Too Many Requests: Rate limiting a...
(QB_NEW_EN)
[grammar] ~226-~226: There might be a mistake here.
Context: ...o Many Requests**: Rate limiting applied - 500 Internal Server Error: Server-side...
(QB_NEW_EN)
oauth2/authorization_code/README.md
[grammar] ~87-~87: There might be a mistake here.
Context: ...rization code and state from the request - Validates the state parameter - Exchange...
(QB_NEW_EN)
[grammar] ~88-~88: There might be a mistake here.
Context: ... request - Validates the state parameter - Exchanges the code for tokens - Stores t...
(QB_NEW_EN)
[grammar] ~89-~89: There might be a mistake here.
Context: ...arameter - Exchanges the code for tokens - Stores the tokens using your session hoo...
(QB_NEW_EN)
[grammar] ~131-~131: There might be a mistake here.
Context: ...ializes the auth flow** for each request 2. Handles the callback at `/kinde/callba...
(QB_NEW_EN)
[grammar] ~132-~132: There might be a mistake here.
Context: ...ack** at /kinde/callback automatically 3. Protects all routes in the group by ch...
(QB_NEW_EN)
[grammar] ~133-~133: There might be a mistake here.
Context: ... in the group by checking authentication 4. Redirects unauthenticated users to the...
(QB_NEW_EN)
[grammar] ~134-~134: There might be a mistake here.
Context: ...ticated users** to the authorization URL 5. Provides the Kinde client in the Gin c...
(QB_NEW_EN)
[grammar] ~177-~177: There might be a mistake here.
Context: ...ntication checking** on protected routes - Seamless redirects to the authorizatio...
(QB_NEW_EN)
[grammar] ~178-~178: There might be a mistake here.
Context: ... redirects** to the authorization server - Session management integration - **Tok...
(QB_NEW_EN)
[grammar] ~179-~179: There might be a mistake here.
Context: ...ver - Session management integration - Token validation and refresh handling ...
(QB_NEW_EN)
[grammar] ~180-~180: There might be a mistake here.
Context: ...Token validation* and refresh handling - Error handling with appropriate HTTP s...
(QB_NEW_EN)
[grammar] ~210-~210: There might be a mistake here.
Context: ...*: Validates token signatures using JWKS - Algorithm Validation: Ensures tokens u...
(QB_NEW_EN)
[grammar] ~211-~211: There might be a mistake here.
Context: ...use the expected algorithm (e.g., RS256) - Audience Validation: Confirms tokens i...
(QB_NEW_EN)
[grammar] ~212-~212: There might be a mistake here.
Context: ... tokens include the correct API audience - Custom Validation: Additional validati...
(QB_NEW_EN)
[style] ~222-~222: This adverb was used twice in the sentence. Consider removing one of them or replacing them with a synonym.
Context: ...tically - Maintain long-term sessions - Automatically refresh expired access tokens ## Examp...
(ADVERB_REPETITION_PREMIUM)
🪛 markdownlint-cli2 (0.17.2)
oauth2/authorization_code/README.md
80-80: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
🔇 Additional comments (3)
README_MANAGEMENT_API.md (1)
23-27: Imports look correct and aligned with the SDK layoutYou’ve imported client_credentials, frameworks/cli, and jwt in one block. This resolves earlier confusion about the CLI hooks path and missing jwt.
README.md (1)
71-79: Good: GetClient error handling addedThe example now captures and checks the error from
GetClient, preventing misleading usage.oauth2/authorization_code/README.md (1)
1-8: Solid high-level framingThe overview correctly positions the flow as a backend/server-side flow with session management outside the browser, which is accurate for authorization_code.
- Introduce `InjectTokenMiddleware` to inject authentication tokens into the request context for downstream handlers. - Add `TokenFromContext` helper function to extract tokens from the context. - Create example usage in `example_middleware.go` demonstrating middleware integration. - Implement unit tests for middleware functionality and token extraction. - Update `README_MIDDLEWARE.md` with detailed usage instructions and best practices for middleware implementation.
There was a problem hiding this comment.
Actionable comments posted: 4
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
oauth2/authorization_code/authorization_code.go (2)
160-166: Bug: Handler writes 500 but still calls SetRawToken with a potentially nil token. Return early on error.If
Exchangefails, the handler should stop; otherwise you may write to the session with a nil/invalid token.Apply this diff:
token, err := flow.config.Exchange(r.Context(), r.URL.Query().Get("code")) if err != nil { http.Error(w, err.Error(), http.StatusInternalServerError) + return } - flow.sessionHooks.SetRawToken(token) + if err := flow.sessionHooks.SetRawToken(token); err != nil { + http.Error(w, err.Error(), http.StatusInternalServerError) + return + }
261-263: Persisting token error is ignored in ExchangeCode — surface it to the caller.If saving the token to the session fails,
ExchangeCodestill returnsnil, which can mislead callers into assuming success.Apply this diff:
- flow.sessionHooks.SetRawToken(token) - - return err + if err := flow.sessionHooks.SetRawToken(token); err != nil { + return fmt.Errorf("failed to persist token in session: %w", err) + } + return nil
🧹 Nitpick comments (4)
oauth2/authorization_code/authorization_code.go (2)
19-19: Centralize the context key to avoid typos and duplicated literals.Define a single, unexported key and reuse it everywhere instead of repeating
contextKey("kinde_token").Apply this diff to introduce a package-level constant:
type ( - contextKey string + contextKey string ) + +// ctxTokenKey is the private context key used to store the Kinde token. +const ctxTokenKey contextKey = "kinde_token"
196-199: Ignore-return without intent: make it explicit or handle the error from SetState.
SetStatecan fail; either handle it or make the intentional ignore explicit to silence linters.Apply this diff:
- flow.sessionHooks.SetState(state) + _ = flow.sessionHooks.SetState(state)oauth2/authorization_code/middleware_test.go (2)
160-166: Use the centralized key for consistency in tests as well.Leverage
ctxTokenKeyinstead of constructingcontextKey("kinde_token")literals in tests.Apply this diff:
- ctx := context.WithValue(context.Background(), contextKey("kinde_token"), mockToken) + ctx := context.WithValue(context.Background(), ctxTokenKey, mockToken)- ctx := context.WithValue(context.Background(), contextKey("kinde_token"), "not_a_token") + ctx := context.WithValue(context.Background(), ctxTokenKey, "not_a_token")Also applies to: 183-186
79-114: This “successfully injects token” test doesn’t exercise the real middleware.It validates context plumbing through a mock, not
AuthorizationCodeFlow.InjectTokenMiddleware. Consider adding a happy-path test for the real middleware (e.g., via a seam to stub token retrieval) once feasible.If you’d like, I can propose a minimal seam (e.g., a tokenSource factory on AuthorizationCodeFlow) to enable a deterministic unit test for the success path.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
⛔ Files ignored due to path filters (1)
go.modis excluded by!**/*.mod
📒 Files selected for processing (4)
oauth2/authorization_code/README_MIDDLEWARE.md(1 hunks)oauth2/authorization_code/authorization_code.go(4 hunks)oauth2/authorization_code/example_middleware.go(1 hunks)oauth2/authorization_code/middleware_test.go(1 hunks)
🧰 Additional context used
🧬 Code graph analysis (3)
oauth2/authorization_code/example_middleware.go (2)
oauth2/authorization_code/authorization_code.go (1)
TokenFromContext(332-335)jwt/jwt.go (1)
Token(22-27)
oauth2/authorization_code/middleware_test.go (2)
jwt/jwt.go (1)
Token(22-27)oauth2/authorization_code/authorization_code.go (2)
TokenFromContext(332-335)AuthorizationCodeFlow(76-88)
oauth2/authorization_code/authorization_code.go (1)
jwt/jwt.go (1)
Token(22-27)
🪛 LanguageTool
oauth2/authorization_code/README_MIDDLEWARE.md
[grammar] ~185-~185: There might be a mistake here.
Context: ...if found, nil otherwise - bool - True if token was found, false otherwise ## To...
(QB_NEW_EN)
[grammar] ~245-~245: There might be a mistake here.
Context: ...` ## Best Practices 1. Always check if token exists: Use the boolean return ...
(QB_NEW_EN)
[grammar] ~245-~245: There might be a mistake here.
Context: ...ts**: Use the boolean return value from TokenFromContext 2. Validate token validity: Check `token....
(QB_NEW_EN)
… function for clarity
There was a problem hiding this comment.
Actionable comments posted: 2
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
README.md (1)
164-167: Remove stray fenced code blocksThese lone ``` fences break Markdown rendering and trigger MD040. Delete them.
-``` - -``` +
♻️ Duplicate comments (6)
oauth2/authorization_code/README_MIDDLEWARE.md (5)
245-250: Best practices: consistency and punctuationAdd periods and keep parallel phrasing.
-1. **Always check if token exists**: Use the boolean return value from `TokenFromContext` -2. **Validate token validity**: Check `token.IsValid()` before using the token -3. **Handle missing tokens gracefully**: Provide fallback behavior for unauthenticated users -4. **Use appropriate HTTP status codes**: Return 401 for missing tokens, 403 for insufficient permissions -5. **Chain middleware carefully**: Apply the Kinde middleware before custom middleware that depends on it +1. **Always check if a token exists**: use the boolean return value from `TokenFromContext`. +2. **Validate token validity**: check `token.IsValid()` before using the token. +3. **Handle missing tokens gracefully**: provide fallback behavior for unauthenticated users. +4. **Use appropriate HTTP status codes**: return 401 for missing tokens; 403 for insufficient permissions. +5. **Chain middleware carefully**: apply the Kinde middleware before custom middleware that depends on it.
81-82: Use InjectTokenMiddleware (flow.Middleware doesn’t exist)Replace flow.Middleware with flow.InjectTokenMiddleware to match the actual API.
- protectedWithAuth := flow.Middleware(protectedMux) + protectedWithAuth := flow.InjectTokenMiddleware(protectedMux)
104-106: Gorilla Mux: swap to InjectTokenMiddlewareThe middleware function to pass into Use should be flow.InjectTokenMiddleware.
- protected.Use(flow.Middleware) + protected.Use(flow.InjectTokenMiddleware)
148-151: Chaining example: update to InjectTokenMiddlewareKeep examples consistent and compilable by using InjectTokenMiddleware.
- adminHandler := RequireScope("admin")(flow.Middleware(adminOnlyHandler)) + adminHandler := RequireScope("admin")(flow.InjectTokenMiddleware(adminOnlyHandler))
160-171: Rename API heading to InjectTokenMiddleware and tighten wordingDocs should reflect the actual exported method name; also end “Returns” sentence with a period.
-#### `Middleware(next http.Handler) http.Handler` +#### `InjectTokenMiddleware(next http.Handler) http.Handler` @@ -**Returns:** - -- An HTTP handler that wraps the original handler with token injection +**Returns:** + +- An HTTP handler that wraps the original handler with token injection.README_MANAGEMENT_API.md (1)
128-136: Make NewManagementAPI example copy-pastable (ctx and imports)-import ( - "github.com/kinde-oss/kinde-go/kinde" -) - -managementApi, err := kinde.NewManagementAPI(ctx, "<kinde domain>", <client credentials flow>) +import ( + "context" + "github.com/kinde-oss/kinde-go/kinde" +) + +ctx := context.Background() +managementApi, err := kinde.NewManagementAPI(ctx, "https://my_kinde_tenant.kinde.com", kindeClient) if err != nil { // Handle error }
🧹 Nitpick comments (11)
oauth2/authorization_code/README_MIDDLEWARE.md (2)
21-24: Add missing import: log is used but not importedExample calls log.Fatal(err) but log isn’t imported.
import ( "net/http" "github.com/kinde-oss/kinde-go/oauth2/authorization_code" + "log" )
182-186: Polish “Returns” formatting and punctuationUse consistent punctuation and style in the TokenFromContext return docs.
-**Returns:** - -- `*jwt.Token` - The token if found, nil otherwise -- `bool` - True if token was found, false otherwise +**Returns:** + +- `*jwt.Token`: the token if found; nil otherwise. +- `bool`: true if a token was found; false otherwise.oauth2/authorization_code/README.md (2)
71-72: Minor grammar: add “the”-| `InjectTokenMiddleware` | Middleware that injects the auth token into request context. | next `http.Handler` | `http.Handler` | +| `InjectTokenMiddleware` | Middleware that injects the auth token into the request context. | next `http.Handler` | `http.Handler` |
245-249: Correct dependency listing: oauth2 isn’t a standard library packageList golang.org/x/oauth2 explicitly and avoid calling it “Standard Go packages.”
-This package requires Go 1.24+ and depends on the following packages: +This package requires Go 1.24+ and depends on: @@ -- Standard Go packages: `context`, `net/http`, `oauth2` +- Standard library: `context`, `net/http` +- External: `golang.org/x/oauth2`README_MANAGEMENT_API.md (3)
23-27: Add missing import: log is used belowimport ( "github.com/kinde-oss/kinde-go/oauth2/client_credentials" "github.com/kinde-oss/kinde-go/frameworks/cli" "github.com/kinde-oss/kinde-go/jwt" + "log" )
79-81: Remove angle brackets around the Management API URLLiteral URLs in code should not be wrapped in <>; also keep capitalization consistent.
- client_credentials.WithKindeManagementAPI("<https://my_kinde_tenant.kinde.com>"), + client_credentials.WithKindeManagementAPI("https://my_kinde_tenant.kinde.com"),
101-103: Memory Session snippet: remove angle brackets and keep formatting consistent- client_credentials.WithKindeManagementAPI("<https://my_kinde_tenant.kinde.com>"), + client_credentials.WithKindeManagementAPI("https://my_kinde_tenant.kinde.com"),README.md (4)
44-52: Typos and URL literal fixes in client credentials snippet
- “optioanlly” → “optionally”
- “aquired” → “acquired”
- Remove angle brackets around the Management API URL
- client_credentials.WithAudience("[your API audience]"), // optioanlly include your API audience + client_credentials.WithAudience("[your API audience]"), // optionally include your API audience client_credentials.WithScopes() // optional - request API scopes - client_credentials.WithKindeManagementAPI("<https://my_kinde_tenant.kinde.com>"), // adds kinde management API audience - see README_MANAGEMENT_API.md for details + client_credentials.WithKindeManagementAPI("https://my_kinde_tenant.kinde.com"), // adds Kinde Management API audience - see README_MANAGEMENT_API.md for details client_credentials.WithSessionHooks(<ISessionHooks implementation>), // example of CLI is cli.NewCliSession(...) - client_credentials.WithTokenValidation( // validates tokens when a new token is aquired + client_credentials.WithTokenValidation( // validates tokens when a new token is acquired
65-67: Fix small typo-Client willl manage tokens in the background, reading/persisting them to provided the session storage. +Client will manage tokens in the background, reading/persisting them to the provided session storage.
78-79: Separate comment and code; wrap the example call in a code blockThe comment and code are on the same line, and the call isn’t fenced, which hurts readability.
-// example call to Kinde Management API (client needs WithKindeManagementAPI(...)) - see README_MANAGEMENT_API.md for details response, err := client.Get("<an authorized URL>") +// Example call to Kinde Management API (client needs WithKindeManagementAPI(...)); see README_MANAGEMENT_API.md for details. +```go +response, err := client.Get("<an authorized URL>") +```
150-151: Fix nested/angled link markupRemove the nested link and angle brackets; use a single, clear link to the Go SDK docs.
-For details on integrating this SDK into your project, head over to the [Kinde docs](https://kinde.com/docs/) and see the [Go SDK](<[link-to-kinde-doc](https://kinde.com/docs/developer-tools/)>) doc 👍🏼. +For details on integrating this SDK into your project, see the Kinde Docs — Go SDK: https://kinde.com/docs/developer-tools/ 👍🏼
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (5)
README.md(6 hunks)README_MANAGEMENT_API.md(1 hunks)oauth2/authorization_code/README.md(1 hunks)oauth2/authorization_code/README_MIDDLEWARE.md(1 hunks)oauth2/authorization_code/example_middleware.go(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- oauth2/authorization_code/example_middleware.go
🧰 Additional context used
🪛 LanguageTool
README.md
[grammar] ~5-~5: There might be a mistake here.
Context: ...EMENT_API.md](README_MANAGEMENT_API.md).
(QB_NEW_EN)
[grammar] ~26-~26: There might be a mistake here.
Context: ...orization code flow for web applications - Device authorization flow for devices wi...
(QB_NEW_EN)
[grammar] ~27-~27: There might be a mistake here.
Context: ... devices with limited input capabilities - Session management and token validation ...
(QB_NEW_EN)
[grammar] ~28-~28: There might be a mistake here.
Context: ... Session management and token validation - Offline support with refresh token manag...
(QB_NEW_EN)
[grammar] ~122-~122: There might be a mistake here.
Context: ...**: oauth2/authorization_code/README.md - Client Credentials Flow: See the Clien...
(QB_NEW_EN)
README_MANAGEMENT_API.md
[grammar] ~11-~11: There might be a mistake here.
Context: ...uisites - A Kinde account with a tenant - A Machine-to-Machine (M2M) application c...
(QB_NEW_EN)
[grammar] ~12-~12: There might be a mistake here.
Context: ...lication configured in your Kinde tenant - Management API access enabled for your M...
(QB_NEW_EN)
[grammar] ~13-~13: There might be a mistake here.
Context: ... access enabled for your M2M application - Appropriate scopes configured for the op...
(QB_NEW_EN)
[grammar] ~86-~86: There might be a mistake here.
Context: ...ger on Windows, Secret Service on Linux) - Automatically handles token chunking for...
(QB_NEW_EN)
[grammar] ~87-~87: There might be a mistake here.
Context: ... handles token chunking for large tokens - Secure by default with proper access con...
(QB_NEW_EN)
[grammar] ~88-~88: There might be a mistake here.
Context: ...e by default with proper access controls - Ideal for command-line tools and desktop...
(QB_NEW_EN)
[grammar] ~107-~107: There might be a mistake here.
Context: ...in memory (lost when process terminates) - Thread-safe with proper locking - Good f...
(QB_NEW_EN)
[grammar] ~108-~108: There might be a mistake here.
Context: ...nates) - Thread-safe with proper locking - Good for testing and short-lived process...
(QB_NEW_EN)
[grammar] ~109-~109: There might be a mistake here.
Context: ...od for testing and short-lived processes - No external dependencies ### Custom Ses...
(QB_NEW_EN)
[grammar] ~118-~118: There might be a mistake here.
Context: ...base-backed storage for web applications - Redis for distributed systems - Encrypte...
(QB_NEW_EN)
[grammar] ~119-~119: There might be a mistake here.
Context: ...ications - Redis for distributed systems - Encrypted file storage - Integration wit...
(QB_NEW_EN)
[grammar] ~120-~120: There might be a mistake here.
Context: ...ributed systems - Encrypted file storage - Integration with existing session manage...
(QB_NEW_EN)
[grammar] ~219-~219: There might be a mistake here.
Context: ... Bad Request**: Invalid input parameters - 401 Unauthorized: Invalid or expired t...
(QB_NEW_EN)
[grammar] ~220-~220: There might be a mistake here.
Context: ...Unauthorized**: Invalid or expired token - 403 Forbidden: Insufficient permission...
(QB_NEW_EN)
[grammar] ~221-~221: There might be a mistake here.
Context: ...nsufficient permissions or missing scope - 429 Too Many Requests: Rate limiting a...
(QB_NEW_EN)
[grammar] ~222-~222: There might be a mistake here.
Context: ...o Many Requests**: Rate limiting applied - 500 Internal Server Error: Server-side...
(QB_NEW_EN)
oauth2/authorization_code/README.md
[grammar] ~70-~70: There might be a mistake here.
Context: ...ntext.Context|(*jwt.Token, error)| |InjectTokenMiddleware` | Middleware t...
(QB_NEW_EN)
[grammar] ~71-~71: There might be a mistake here.
Context: ... Middleware that injects the auth token into request context. | next http.Handler ...
(QB_NEW_EN)
[grammar] ~90-~90: There might be a mistake here.
Context: ...rization code and state from the request - Validates the state parameter - Exchange...
(QB_NEW_EN)
[grammar] ~91-~91: There might be a mistake here.
Context: ... request - Validates the state parameter - Exchanges the code for tokens - Stores t...
(QB_NEW_EN)
[grammar] ~92-~92: There might be a mistake here.
Context: ...arameter - Exchanges the code for tokens - Stores the tokens using your session hoo...
(QB_NEW_EN)
[grammar] ~134-~134: There might be a mistake here.
Context: ...ializes the auth flow** for each request 2. Handles the callback at `/kinde/callba...
(QB_NEW_EN)
[grammar] ~135-~135: There might be a mistake here.
Context: ...ack** at /kinde/callback automatically 3. Protects all routes in the group by ch...
(QB_NEW_EN)
[grammar] ~136-~136: There might be a mistake here.
Context: ... in the group by checking authentication 4. Redirects unauthenticated users to the...
(QB_NEW_EN)
[grammar] ~137-~137: There might be a mistake here.
Context: ...ticated users** to the authorization URL 5. Provides the Kinde client in the Gin c...
(QB_NEW_EN)
[grammar] ~180-~180: There might be a mistake here.
Context: ...ntication checking** on protected routes - Seamless redirects to the authorizatio...
(QB_NEW_EN)
[grammar] ~181-~181: There might be a mistake here.
Context: ... redirects** to the authorization server - Session management integration - **Tok...
(QB_NEW_EN)
[grammar] ~182-~182: There might be a mistake here.
Context: ...ver - Session management integration - Token validation and refresh handling ...
(QB_NEW_EN)
[grammar] ~183-~183: There might be a mistake here.
Context: ...Token validation* and refresh handling - Error handling with appropriate HTTP s...
(QB_NEW_EN)
[grammar] ~213-~213: There might be a mistake here.
Context: ...*: Validates token signatures using JWKS - Algorithm Validation: Ensures tokens u...
(QB_NEW_EN)
[grammar] ~214-~214: There might be a mistake here.
Context: ...use the expected algorithm (e.g., RS256) - Audience Validation: Confirms tokens i...
(QB_NEW_EN)
[grammar] ~215-~215: There might be a mistake here.
Context: ... tokens include the correct API audience - Custom Validation: Additional validati...
(QB_NEW_EN)
[style] ~225-~225: This adverb was used twice in the sentence. Consider removing one of them or replacing them with a synonym.
Context: ...tically - Maintain long-term sessions - Automatically refresh expired access tokens ## Examp...
(ADVERB_REPETITION_PREMIUM)
oauth2/authorization_code/README_MIDDLEWARE.md
[grammar] ~185-~185: There might be a mistake here.
Context: ...if found, nil otherwise - bool - True if token was found, false otherwise ## To...
(QB_NEW_EN)
[grammar] ~245-~245: There might be a mistake here.
Context: ...` ## Best Practices 1. Always check if token exists: Use the boolean return ...
(QB_NEW_EN)
[grammar] ~245-~245: There might be a mistake here.
Context: ...ts**: Use the boolean return value from TokenFromContext 2. Validate token validity: Check `token....
(QB_NEW_EN)
🪛 markdownlint-cli2 (0.17.2)
README.md
164-164: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
oauth2/authorization_code/README.md
61-61: Multiple headings with the same content
(MD024, no-duplicate-heading)
🔇 Additional comments (1)
README.md (1)
70-75: Nice improvement: proper error handling for GetClientThe updated example now checks and handles the error returned by GetClient. Good callout for users.
…cking logic in middleware
… update logic to check for "blue" as the valid favorite color in README.md
There was a problem hiding this comment.
Actionable comments posted: 2
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (3)
README.md (2)
135-135: Fix malformed link syntaxThe nested/angle-bracketed link is malformed and renders incorrectly.
-For details on integrating this SDK into your project, head over to the [Kinde docs](https://kinde.com/docs/) and see the [Go SDK](<[link-to-kinde-doc](https://kinde.com/docs/developer-tools/)>) doc 👍🏼. +For details on integrating this SDK into your project, head over to the [Kinde docs](https://kinde.com/docs/) and see the [Go SDK](https://kinde.com/docs/developer-tools/) doc 👍🏼.
148-152: Remove stray backtick fences at EOFThere are two dangling code-fence closers that will break Markdown rendering.
- -``` - -``` +oauth2/authorization_code/README.md (1)
245-247: Fix dependencies note: oauth2 is not a standard library packageClarify that
oauth2is fromgolang.org/x/oauth2and avoid calling it a standard package.-- Standard Go packages: `context`, `net/http`, `oauth2` +- Standard Go packages: `context`, `net/http` +- External: `golang.org/x/oauth2`
♻️ Duplicate comments (5)
README.md (1)
69-76: Good fix: now handling GetClient error correctlyThanks for updating the example to capture and handle the
GetClienterror; this aligns with the function’s signature and improves developer guidance.oauth2/authorization_code/README_MIDDLEWARE.md (4)
81-83: Use InjectTokenMiddleware (flow.Middleware does not exist)The API provides
InjectTokenMiddleware. Update this usage to avoid confusing readers.- // Apply middleware to all protected routes - protectedWithAuth := flow.Middleware(protectedMux) + // Apply middleware to all protected routes + protectedWithAuth := flow.InjectTokenMiddleware(protectedMux)
104-106: Replace protected.Use(flow.Middleware) with InjectTokenMiddlewareGorilla Mux section should reference the actual middleware.
- protected.Use(flow.Middleware) + protected.Use(flow.InjectTokenMiddleware)
158-161: Chain RequireScope with InjectTokenMiddleware, not MiddlewareKeep middleware naming consistent with the public API.
- adminHandler := RequireScope("admin")(flow.Middleware(adminOnlyHandler)) + adminHandler := RequireScope("admin")(flow.InjectTokenMiddleware(adminOnlyHandler))
170-181: Rename API reference to InjectTokenMiddleware and finalize punctuationMatch the actual API surface and tidy return docs.
-#### `Middleware(next http.Handler) http.Handler` +#### `InjectTokenMiddleware(next http.Handler) http.Handler` @@ -Creates middleware that injects the current user's token into the request context. +Creates middleware that injects the current user's token into the request context. @@ -**Returns:** - -- An HTTP handler that wraps the original handler with token injection +**Returns:** + +- An HTTP handler that wraps the original handler with token injection.
🧹 Nitpick comments (7)
README.md (1)
44-52: Fix typos and tighten comments in Client Credentials exampleMinor nits that improve readability and professionalism.
- client_credentials.WithAudience("[your API audience]"), // optioanlly include your API audience + client_credentials.WithAudience("[your API audience]"), // optionally include your API audience client_credentials.WithScopes() // optional - request API scopes client_credentials.WithKindeManagementAPI("<https://my_kinde_tenant.kinde.com>"), // adds kinde management API audience - see README_MANAGEMENT_API.md for details client_credentials.WithSessionHooks(<ISessionHooks implementation>), // example of CLI is cli.NewCliSession(...) - client_credentials.WithTokenValidation( // validates tokens when a new token is aquired + client_credentials.WithTokenValidation( // validates tokens when a new token is acquired true, // will validate token signature via JWKS jwt.WillValidateAlgorithm(), // will validate the token alg is RS256 - jwt.WillValidateAudience("<your API audience>"), // will confirm that received token includes correct audience + jwt.WillValidateAudience("<your API audience>"), // will confirm that received token includes correct audience ),oauth2/authorization_code/README_MIDDLEWARE.md (4)
21-25: Add missing import for log in the basic exampleThe example uses
log.Fatalbut doesn’t importlog. Add it to the import list.import ( + "log" "net/http" "github.com/kinde-oss/kinde-go/oauth2/authorization_code" )Also applies to: 34-36
141-147: Prefer strings.Fields for scope parsing and note the import
strings.Split(scopeStr, " ")can produce empty elements with irregular whitespace.strings.Fieldsis more robust. Also make sureimport "strings"is present where this snippet lives.- if scopeStr, ok := claims["scope"].(string); ok { - for _, s := range strings.Split(scopeStr, " ") { + if scopeStr, ok := claims["scope"].(string); ok { + for _, s := range strings.Fields(scopeStr) { if s == requiredScope { next.ServeHTTP(w, r) return } } }Outside this snippet, add:
import "strings"
193-196: Tighten return description formattingMinor polish: use consistent punctuation and phrasing.
- - `*jwt.Token` - The token if found, nil otherwise - - `bool` - True if token was found, false otherwise + - `*jwt.Token`: the token if found; nil otherwise. + - `bool`: true if a token was found; false otherwise.
255-260: Best practices: end bullets with periodsConsistent punctuation throughout improves readability.
-1. **Always check if token exists**: Use the boolean return value from `TokenFromContext` -2. **Validate token validity**: Check `token.IsValid()` before using the token -3. **Handle missing tokens gracefully**: Provide fallback behavior for unauthenticated users -4. **Use appropriate HTTP status codes**: Return 401 for missing tokens, 403 for insufficient permissions -5. **Chain middleware carefully**: Apply the Kinde middleware before custom middleware that depends on it +1. **Always check if a token exists**: use the boolean return value from `TokenFromContext`. +2. **Validate token validity**: check `token.IsValid()` before using the token. +3. **Handle missing tokens gracefully**: provide fallback behavior for unauthenticated users. +4. **Use appropriate HTTP status codes**: return 401 for missing tokens; 403 for insufficient permissions. +5. **Chain middleware carefully**: apply the Kinde middleware before custom middleware that depends on it.oauth2/authorization_code/README.md (2)
98-128: Gin example: add imports and use a concrete sessions storeAs written, the snippet omits imports for
ginandsessionsand references a non-existentsessions.NewStore(...). Suggest a minimal working example using the cookie store:-import "github.com/kinde-oss/kinde-go/frameworks/gin_kinde" +import ( + "github.com/gin-contrib/sessions" + "github.com/gin-contrib/sessions/cookie" + "github.com/gin-gonic/gin" + "github.com/kinde-oss/kinde-go/frameworks/gin_kinde" +) @@ - store := sessions.NewStore(...) + store := cookie.NewStore([]byte("replace-with-strong-secret"))
188-199: WithTokenValidation signature: include validators or confirm boolean-only formElsewhere in the docs you show
WithTokenValidation(true, jwt.WillValidateAlgorithm(), ...). Here it’s called with only a boolean. If both forms are supported, add a comment clarifying the defaults; if not, extend the example to include at least one validator.Example if validators are required:
- authorization_code.WithTokenValidation(true), // Enable token validation + authorization_code.WithTokenValidation( + true, // enable signature validation via JWKS + jwt.WillValidateAlgorithm(), // enforce RS256 by default + ),
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (4)
README.md(6 hunks)jwt/README.md(1 hunks)oauth2/authorization_code/README.md(1 hunks)oauth2/authorization_code/README_MIDDLEWARE.md(1 hunks)
🧰 Additional context used
🪛 LanguageTool
README.md
[grammar] ~5-~5: There might be a mistake here.
Context: ...EMENT_API.md](README_MANAGEMENT_API.md).
(QB_NEW_EN)
[grammar] ~26-~26: There might be a mistake here.
Context: ...orization code flow for web applications - Device authorization flow for devices wi...
(QB_NEW_EN)
[grammar] ~27-~27: There might be a mistake here.
Context: ... devices with limited input capabilities - Session management and token validation ...
(QB_NEW_EN)
[grammar] ~28-~28: There might be a mistake here.
Context: ... Session management and token validation - Offline support with refresh token manag...
(QB_NEW_EN)
[grammar] ~107-~107: There might be a mistake here.
Context: ...**: oauth2/authorization_code/README.md - Client Credentials Flow: See the Clien...
(QB_NEW_EN)
oauth2/authorization_code/README.md
[grammar] ~68-~68: There might be a mistake here.
Context: ...ntext.Context|(*jwt.Token, error)| |InjectTokenMiddleware` | Middleware t...
(QB_NEW_EN)
[grammar] ~69-~69: There might be a mistake here.
Context: ... Middleware that injects the auth token into request context. | next http.Handler ...
(QB_NEW_EN)
[grammar] ~88-~88: There might be a mistake here.
Context: ...rization code and state from the request - Validates the state parameter - Exchange...
(QB_NEW_EN)
[grammar] ~89-~89: There might be a mistake here.
Context: ... request - Validates the state parameter - Exchanges the code for tokens - Stores t...
(QB_NEW_EN)
[grammar] ~90-~90: There might be a mistake here.
Context: ...arameter - Exchanges the code for tokens - Stores the tokens using your session hoo...
(QB_NEW_EN)
[grammar] ~132-~132: There might be a mistake here.
Context: ...ializes the auth flow** for each request 2. Handles the callback at `/kinde/callba...
(QB_NEW_EN)
[grammar] ~133-~133: There might be a mistake here.
Context: ...ack** at /kinde/callback automatically 3. Protects all routes in the group by ch...
(QB_NEW_EN)
[grammar] ~134-~134: There might be a mistake here.
Context: ... in the group by checking authentication 4. Redirects unauthenticated users to the...
(QB_NEW_EN)
[grammar] ~135-~135: There might be a mistake here.
Context: ...ticated users** to the authorization URL 5. Provides the Kinde client in the Gin c...
(QB_NEW_EN)
[grammar] ~178-~178: There might be a mistake here.
Context: ...ntication checking** on protected routes - Seamless redirects to the authorizatio...
(QB_NEW_EN)
[grammar] ~179-~179: There might be a mistake here.
Context: ... redirects** to the authorization server - Session management integration - **Tok...
(QB_NEW_EN)
[grammar] ~180-~180: There might be a mistake here.
Context: ...ver - Session management integration - Token validation and refresh handling ...
(QB_NEW_EN)
[grammar] ~181-~181: There might be a mistake here.
Context: ...Token validation* and refresh handling - Error handling with appropriate HTTP s...
(QB_NEW_EN)
[grammar] ~211-~211: There might be a mistake here.
Context: ...*: Validates token signatures using JWKS - Algorithm Validation: Ensures tokens u...
(QB_NEW_EN)
[grammar] ~212-~212: There might be a mistake here.
Context: ...use the expected algorithm (e.g., RS256) - Audience Validation: Confirms tokens i...
(QB_NEW_EN)
[grammar] ~213-~213: There might be a mistake here.
Context: ... tokens include the correct API audience - Custom Validation: Additional validati...
(QB_NEW_EN)
[style] ~223-~223: This adverb was used twice in the sentence. Consider removing one of them or replacing them with a synonym.
Context: ...tically - Maintain long-term sessions - Automatically refresh expired access tokens ## Examp...
(ADVERB_REPETITION_PREMIUM)
oauth2/authorization_code/README_MIDDLEWARE.md
[grammar] ~195-~195: There might be a mistake here.
Context: ...if found, nil otherwise - bool - True if token was found, false otherwise ## To...
(QB_NEW_EN)
[grammar] ~255-~255: There might be a mistake here.
Context: ...` ## Best Practices 1. Always check if token exists: Use the boolean return ...
(QB_NEW_EN)
[grammar] ~255-~255: There might be a mistake here.
Context: ...ts**: Use the boolean return value from TokenFromContext 2. Validate token validity: Check `token....
(QB_NEW_EN)
🔇 Additional comments (1)
oauth2/authorization_code/README.md (1)
63-70: Confirm whether InjectTokenMiddleware belongs in the Device Flow API table
InjectTokenMiddlewareis typically associated with the authorization-code flow and context-injected sessions. If device flow does not expose this method, listing it here could mislead users.Please verify the interface(s) and either:
- move
InjectTokenMiddlewareto the appropriate flow’s methods table, or- confirm that the device flow also implements it and add a brief note.
1 participant
Introduce memory session hooks for managing OAuth2 client credentials flow, ensuring thread safety and error handling. Add comprehensive documentation for the Kinde Management API, improving accessibility and clarity for users. Update main README to link to the new documentation.