feat: swagger#973
Open
steveiliop56 wants to merge 6 commits into
Open
Conversation
Contributor
📝 WalkthroughWalkthroughThis PR adds Scalar-based API documentation served at /scalar (gated by a new ScalarEnabled config flag), generates Swagger/OpenAPI specs (docs.go, swagger.json, swagger.yaml), introduces typed JSON response structs (SimpleResponse, OAuthURLSuccessResponse, AuthorizeCompleteResponse, OIDCErrorResponse, TotpPendingResponse) replacing gin.H maps across controllers, adds Swagger annotations to handlers, changes OAuth callback redirects from StatusTemporaryRedirect to StatusFound, and updates build/dependency tooling. ChangesSwagger/Scalar Documentation and Typed Responses
Estimated code review effort: 4 (Complex) | ~60 minutes Sequence Diagram(s)sequenceDiagram
participant Browser
participant Server
participant ScalarUI
Browser->>Server: GET /scalar/*any
Server->>Server: check config.Server.ScalarEnabled
alt Scalar enabled
Server->>ScalarUI: WrapHandler(nil) serves docs
ScalarUI-->>Browser: Render API documentation
else Scalar disabled
Server-->>Browser: route not registered
end
sequenceDiagram
participant Browser
participant OAuthController
participant IdentityProvider
Browser->>OAuthController: GET /api/oauth/callback/{id}
OAuthController->>IdentityProvider: exchange code, fetch userinfo
IdentityProvider-->>OAuthController: token/userinfo or error
alt success
OAuthController-->>Browser: 302 Found redirect to /continue or /oidc/authorize
else failure
OAuthController-->>Browser: 302 Found redirect to {AppURL}/error
end
Possibly related PRs
Suggested reviewers: 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 inconclusive)
✅ Passed checks (4 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
dosubot
Bot
added
the
size:XXL
Codecov Report❌ Patch coverage is 📢 Thoughts on this report? Let us know! |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 3
🧹 Nitpick comments (3)
.env.example (1)
34-36: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winReorder key per dotenv-linter.
dotenv-linter flags
TINYAUTH_SERVER_SCALARENABLEDas out of order relative toTINYAUTH_SERVER_SOCKETPATH.🔧 Proposed fix
-TINYAUTH_SERVER_SOCKETPATH= -# Enable API docs with Scalar under /scalar. -TINYAUTH_SERVER_SCALARENABLED=true +# Enable API docs with Scalar under /scalar. +TINYAUTH_SERVER_SCALARENABLED=true +TINYAUTH_SERVER_SOCKETPATH=🤖 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 @.env.example around lines 34 - 36, The dotenv key order in the example env file is incorrect, causing dotenv-linter to flag TINYAUTH_SERVER_SCALARENABLED as out of order. Reorder the entries in the .env.example section so TINYAUTH_SERVER_SCALARENABLED is placed according to the expected alphabetical/key order relative to TINYAUTH_SERVER_SOCKETPATH, keeping the surrounding configuration comments intact.Source: Linters/SAST tools
internal/bootstrap/router_bootstrap.go (1)
152-154: 🩺 Stability & Availability | 🔵 Trivial | 💤 Low valueGlobal
docs.SwaggerInfomutation on every bootstrap call.
setupScalar()writes to the package-leveldocs.SwaggerInfovar without synchronization. Harmless for a single productionmain()invocation, but if any test or tool creates multipleBootstrapAppinstances concurrently, this is an unsynchronized shared-state write.🤖 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 `@internal/bootstrap/router_bootstrap.go` around lines 152 - 154, setupScalar() is mutating the package-level docs.SwaggerInfo shared state on every BootstrapApp bootstrap, which is unsafe if multiple instances run concurrently. Update the setupScalar flow to avoid writing directly to docs.SwaggerInfo.Host, docs.SwaggerInfo.Schemes, and docs.SwaggerInfo.Version; instead build per-app Swagger metadata locally or guard the shared mutation with synchronization. Use setupScalar and the docs.SwaggerInfo assignments as the main locations to refactor.internal/controller/user_controller.go (1)
65-78: 🗄️ Data Integrity & Integration | 🔵 Trivial | ⚡ Quick winDuplicate
@Success 200annotations produce misleading generated docs.
loginHandlerdocuments two@Success 200responses (SimpleResponseandTotpPendingResponse), but swaggo/swag only keeps the last one when generating the spec for a given status code. The providedswagger.jsonsnippet confirms this:/api/user/login200 only referencescontroller.TotpPendingResponse, even though a plain successful login (line 247) actually returnsSimpleResponsewithout atotpPendingfield. API consumers relying on the generated docs will get an inaccurate contract for the common (non-TOTP) success case.Consider normalizing both success paths to return the same shape (e.g., always include
totpPending, defaulting tofalse), or drop one of the duplicate@Success 200lines and note the alternate shape in the description, since swag has no native support for multiple schemas per status code.♻️ Example: unify the success response shape
c.JSON(200, SimpleResponse{ - Status: 200, - Message: "Login successful", + Status: 200, + Message: "Login successful", })Alternatively, use
TotpPendingResponse{SimpleResponse: SimpleResponse{Status: 200, Message: "Login successful"}, TotpPending: false}here so both branches share one documented type.Also applies to: 190-196, 247-250
🤖 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 `@internal/controller/user_controller.go` around lines 65 - 78, The loginHandler Swagger docs currently declare two different `@Success` 200 schemas, but swaggo only preserves one, so the generated spec is inaccurate for the normal login path. Update the annotations on loginHandler to document a single 200 response shape that matches both branches, ideally by using TotpPendingResponse consistently (with totpPending defaulting to false in the non-TOTP success path) or by removing the duplicate `@Success` 200 and describing the alternate case in the summary/description.
🤖 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.
Inline comments:
In `@internal/bootstrap/router_bootstrap.go`:
- Around line 145-154: setupScalar currently parses app.runtime.AppURL without
rejecting schemeless values, which can leave docs.SwaggerInfo.Schemes empty and
break Scalar/OpenAPI generation. Update setupScalar in BootstrapApp to validate
the parsed URL has a non-empty scheme before assigning docs.SwaggerInfo.Host and
docs.SwaggerInfo.Schemes, and return an error for values like example.com. Use
the existing setupScalar, app.runtime.AppURL, and docs.SwaggerInfo assignments
to locate the fix.
In `@internal/controller/oauth_controller.go`:
- Around line 149-154: The OAuth callback Swagger annotations in the controller
have the `code` and `state` descriptions reversed, so update the `@Param`
comments on the callback endpoint to match the correct meanings. Keep the
parameter names as-is, but swap the description strings in the annotation block
associated with the OAuth callback handler so Scalar/OpenAPI documents `code` as
the authorization code and `state` as the state value.
In `@internal/model/config.go`:
- Around line 37-39: The default configuration in Config currently enables
Scalar by default, which exposes the `/scalar` docs UI publicly through the main
router. Update the Config defaults so ScalarEnabled is false unless explicitly
turned on, and ensure any code path that uses Config or ScalarEnabled still
works with the new default. If the public exposure is intentional, add a brief
note in the relevant docs; otherwise keep the change limited to the config
default.
---
Nitpick comments:
In @.env.example:
- Around line 34-36: The dotenv key order in the example env file is incorrect,
causing dotenv-linter to flag TINYAUTH_SERVER_SCALARENABLED as out of order.
Reorder the entries in the .env.example section so TINYAUTH_SERVER_SCALARENABLED
is placed according to the expected alphabetical/key order relative to
TINYAUTH_SERVER_SOCKETPATH, keeping the surrounding configuration comments
intact.
In `@internal/bootstrap/router_bootstrap.go`:
- Around line 152-154: setupScalar() is mutating the package-level
docs.SwaggerInfo shared state on every BootstrapApp bootstrap, which is unsafe
if multiple instances run concurrently. Update the setupScalar flow to avoid
writing directly to docs.SwaggerInfo.Host, docs.SwaggerInfo.Schemes, and
docs.SwaggerInfo.Version; instead build per-app Swagger metadata locally or
guard the shared mutation with synchronization. Use setupScalar and the
docs.SwaggerInfo assignments as the main locations to refactor.
In `@internal/controller/user_controller.go`:
- Around line 65-78: The loginHandler Swagger docs currently declare two
different `@Success` 200 schemas, but swaggo only preserves one, so the generated
spec is inaccurate for the normal login path. Update the annotations on
loginHandler to document a single 200 response shape that matches both branches,
ideally by using TotpPendingResponse consistently (with totpPending defaulting
to false in the non-TOTP success path) or by removing the duplicate `@Success` 200
and describing the alternate case in the summary/description.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro Plus
Run ID: 242c2f70-da0e-4ff1-be69-54d7c5eabf0f
⛔ Files ignored due to path filters (1)
go.sumis excluded by!**/*.sum
📒 Files selected for processing (20)
.env.exampleMakefilefrontend/vite.config.tsgo.modinternal/bootstrap/router_bootstrap.gointernal/controller/context_controller.gointernal/controller/controller.gointernal/controller/health_controller.gointernal/controller/health_controller_test.gointernal/controller/oauth_controller.gointernal/controller/oidc_controller.gointernal/controller/proxy_controller.gointernal/controller/resources_controller.gointernal/controller/user_controller.gointernal/controller/well_known_controller.gointernal/middleware/ui_middleware.gointernal/model/config.gointernal/swagger/docs.gointernal/swagger/swagger.jsoninternal/swagger/swagger.yaml
Comment on lines
+145
to
+154
| func (app *BootstrapApp) setupScalar() error { | ||
| appUrl, err := url.Parse(app.runtime.AppURL) | ||
|
|
||
| if err != nil { | ||
| return fmt.Errorf("failed to parse app url: %w", err) | ||
| } | ||
|
|
||
| docs.SwaggerInfo.Host = appUrl.Host | ||
| docs.SwaggerInfo.Schemes = []string{appUrl.Scheme} | ||
| docs.SwaggerInfo.Version = model.Version |
Contributor
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Find where AppURL is set/validated
rg -nP -C4 '\bAppURL\b' --type=goRepository: tinyauthapp/tinyauth
Length of output: 158
🏁 Script executed:
#!/bin/bash
set -euo pipefail
printf '\n== bootstrap file context ==\n'
sed -n '1,240p' internal/bootstrap/router_bootstrap.go
printf '\n== search for AppURL-like config fields ==\n'
rg -n --hidden --glob '!**/vendor/**' 'AppURL|appURL|AppUrl|APP_URL|APPURL' .
printf '\n== search for runtime/config loading paths ==\n'
rg -n --hidden --glob '!**/vendor/**' 'runtime\.|NewRuntime|LoadConfig|config.*url|url\.Parse|Parse\(' internal .Repository: tinyauthapp/tinyauth
Length of output: 39472
🏁 Script executed:
#!/bin/bash
set -euo pipefail
printf '\n== app bootstrap AppURL handling ==\n'
sed -n '80,110p' internal/bootstrap/app_bootstrap.go
printf '\n== config model definition ==\n'
sed -n '90,120p' internal/model/config.go
printf '\n== any explicit AppURL validation ==\n'
rg -n --hidden --glob '!**/vendor/**' 'AppURL.*(validate|required|scheme|parse)|scheme.*AppURL|url\.Parse\(app\.config\.AppURL\)' internalRepository: tinyauthapp/tinyauth
Length of output: 3192
Reject schemeless AppURL before populating Scalar docs
setupScalar() assumes app.runtime.AppURL already has a scheme, but the bootstrap path only checks for non-empty and parses app.config.AppURL. A value like example.com will leave docs.SwaggerInfo.Schemes as []string{""} and break the Scalar/OpenAPI URL.
🤖 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 `@internal/bootstrap/router_bootstrap.go` around lines 145 - 154, setupScalar
currently parses app.runtime.AppURL without rejecting schemeless values, which
can leave docs.SwaggerInfo.Schemes empty and break Scalar/OpenAPI generation.
Update setupScalar in BootstrapApp to validate the parsed URL has a non-empty
scheme before assigning docs.SwaggerInfo.Host and docs.SwaggerInfo.Schemes, and
return an error for values like example.com. Use the existing setupScalar,
app.runtime.AppURL, and docs.SwaggerInfo assignments to locate the fix.
Comment on lines
+149
to
+154
| // @Param id path string true "Provider ID" | ||
| // @Param code query string true "State" | ||
| // @Param state query string true "Code" | ||
| // @Success 302 | ||
| // @Failure 302 | ||
| // @Router /api/oauth/callback/{id} [get] |
Contributor
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
Swap the code and state descriptions.
The parameter names are correct, but the descriptions are reversed, so Scalar/OpenAPI will label the callback query params incorrectly.
🛠️ Suggested fix
-// `@Param` code query string true "State"
-// `@Param` state query string true "Code"
+// `@Param` code query string true "Code"
+// `@Param` state query string true "State"📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| // @Param id path string true "Provider ID" | |
| // @Param code query string true "State" | |
| // @Param state query string true "Code" | |
| // @Success 302 | |
| // @Failure 302 | |
| // @Router /api/oauth/callback/{id} [get] | |
| // `@Param` id path string true "Provider ID" | |
| // `@Param` code query string true "Code" | |
| // `@Param` state query string true "State" | |
| // `@Success` 302 | |
| // `@Failure` 302 | |
| // `@Router` /api/oauth/callback/{id} [get] |
🤖 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 `@internal/controller/oauth_controller.go` around lines 149 - 154, The OAuth
callback Swagger annotations in the controller have the `code` and `state`
descriptions reversed, so update the `@Param` comments on the callback endpoint
to match the correct meanings. Keep the parameter names as-is, but swap the
description strings in the annotation block associated with the OAuth callback
handler so Scalar/OpenAPI documents `code` as the authorization code and `state`
as the state value.
Comment on lines
+37
to
+39
| Port: 3000, | ||
| Address: "0.0.0.0", | ||
| ScalarEnabled: true, |
Contributor
There was a problem hiding this comment.
🔒 Security & Privacy | 🟡 Minor | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Map relevant files and inspect the config/default/router/middleware sections.
git ls-files internal/model/config.go internal/bootstrap/router_bootstrap.go internal/bootstrap/ui_middleware.go
echo '--- config.go outline ---'
ast-grep outline internal/model/config.go --view expanded || true
echo '--- router_bootstrap.go outline ---'
ast-grep outline internal/bootstrap/router_bootstrap.go --view expanded || true
echo '--- ui_middleware.go outline ---'
ast-grep outline internal/bootstrap/ui_middleware.go --view expanded || true
echo '--- config.go relevant lines ---'
sed -n '1,220p' internal/model/config.go | cat -n | sed -n '1,220p'
echo '--- router_bootstrap.go relevant lines ---'
sed -n '1,240p' internal/bootstrap/router_bootstrap.go | cat -n | sed -n '1,240p'
echo '--- ui_middleware.go relevant lines ---'
sed -n '1,220p' internal/bootstrap/ui_middleware.go | cat -n | sed -n '1,220p'Repository: tinyauthapp/tinyauth
Length of output: 38419
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Search for scalar-related config and route handling.
rg -n "ScalarEnabled|/scalar|scalar" internal/model internal/bootstrapRepository: tinyauthapp/tinyauth
Length of output: 981
🏁 Script executed:
#!/bin/bash
set -euo pipefail
git ls-files internal/model/config.go internal/bootstrap/router_bootstrap.go internal/bootstrap/ui_middleware.go
echo '--- config.go outline ---'
ast-grep outline internal/model/config.go --view expanded || true
echo '--- router_bootstrap.go outline ---'
ast-grep outline internal/bootstrap/router_bootstrap.go --view expanded || true
echo '--- ui_middleware.go outline ---'
ast-grep outline internal/bootstrap/ui_middleware.go --view expanded || true
echo '--- config.go relevant lines ---'
sed -n '1,220p' internal/model/config.go | cat -n | sed -n '1,220p'
echo '--- router_bootstrap.go relevant lines ---'
sed -n '1,240p' internal/bootstrap/router_bootstrap.go | cat -n | sed -n '1,240p'
echo '--- ui_middleware.go relevant lines ---'
sed -n '1,220p' internal/bootstrap/ui_middleware.go | cat -n | sed -n '1,220p'Repository: tinyauthapp/tinyauth
Length of output: 38419
🏁 Script executed:
#!/bin/bash
set -euo pipefail
rg -n "ScalarEnabled|/scalar|scalar" internal/model internal/bootstrapRepository: tinyauthapp/tinyauth
Length of output: 981
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Find the UI/auth middleware files and inspect route protection around /scalar.
git ls-files internal/middleware internal/bootstrap | sed -n '1,200p'
echo '--- middleware outlines ---'
for f in $(git ls-files internal/middleware/*.go); do
echo "### $f"
ast-grep outline "$f" --view expanded || true
done
echo '--- search auth/scalar/router usage ---'
rg -n "New.*Auth|AuthMiddleware|ShouldBind|SetTrustedProxies|/scalar|ScalarEnabled|Use\\(" internal/middleware internal/bootstrap internal/controllerRepository: tinyauthapp/tinyauth
Length of output: 5221
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo '--- context_middleware.go relevant lines ---'
sed -n '1,380p' internal/middleware/context_middleware.go | cat -n | sed -n '1,380p'
echo '--- ui_middleware.go relevant lines ---'
sed -n '1,220p' internal/middleware/ui_middleware.go | cat -n | sed -n '1,220p'Repository: tinyauthapp/tinyauth
Length of output: 13652
Default /scalar to off ScalarEnabled is true by default, and /scalar is mounted on the main router with no auth gate, so every deployment exposes the docs UI publicly. If that’s intentional, add a short note in the docs; otherwise default it to false.
🤖 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 `@internal/model/config.go` around lines 37 - 39, The default configuration in
Config currently enables Scalar by default, which exposes the `/scalar` docs UI
publicly through the main router. Update the Config defaults so ScalarEnabled is
false unless explicitly turned on, and ensure any code path that uses Config or
ScalarEnabled still works with the new default. If the public exposure is
intentional, add a brief note in the relevant docs; otherwise keep the change
limited to the config default.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary by CodeRabbit
New Features
/scalar.Bug Fixes
Chores