Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
24 changes: 24 additions & 0 deletions .agents/project/conventions.md
Original file line number Diff line number Diff line change
Expand Up @@ -165,6 +165,30 @@ The async task queue is goque. Tasks are registered by type in
must key by user and therefore must sit after the token gate), which is not
observable in either layer alone.

- Do not write tests about configuration plumbing. Same principle as route
wiring, one layer down: a test that builds an `AppConfig` literal and asserts
a predicate over it restates the predicate in a second syntax. That includes
tables enumerating which combinations of keys switch a feature on, and tests
that a defaulted value defaults.

A misconfigured instance announces itself the moment it runs: the feature is
off, the endpoint 404s, the process refuses to boot. Test the *derivation*
that has somewhere to be wrong -- a URL rewritten into a cookie scope, a path
prefix stripped by a proxy -- as a pure function on its inputs, in the package
that owns it. `absoluteURL(FrontendURL)` is worth pinning; `Enabled() == true
when both keys are set` is not.

- Do not add logging to describe configuration back to the operator. A startup
line that prints a value the operator just typed into a file they are looking
at tells them nothing they cannot read faster from the file, and it goes stale
silently when the key is renamed. Log what the process *did* -- a connection
established, a processor registered, a credential resolved from the store --
not what it was told to do.

The exception is a value the process *derived* and the operator cannot
predict: a resolved hostname, a computed path, a generated password. If the
operator can grep it out of a config file, it does not need a log line.

## Generated Code

- Do not manually edit generated files unless the generator output itself is
Expand Down
49 changes: 42 additions & 7 deletions cmd/maintmode/main.go
Original file line number Diff line number Diff line change
Expand Up @@ -45,8 +45,43 @@ import (
"github.com/ruko1202/maintmode/internal/utils/xecho"

"github.com/ruko1202/maintmode/internal/config"
googleoauthgw "github.com/ruko1202/maintmode/internal/gateways/googleoauth"
"github.com/ruko1202/maintmode/internal/storages/oauthdance"
)

// newAuthHandlers builds the auth API component, attaching the backend OAuth
// dance only when it is configured.
//
// The gate is checked here as well as at route registration, and the redundancy
// is deliberate: registration decides whether the endpoints exist, this decides
// whether a token-exchange client holding a client secret is constructed at all.
// An unconfigured instance ends up with neither.
func newAuthHandlers(cfg *config.AppConfig, services *bootstrap.Services, valkeyClient *valkeylib.Client) *apiauth.Implementation {
impl := apiauth.New(
cfg.Auth,
services.Auth,
services.Token,
services.User,
services.OTP,
)

if !cfg.OAuthDanceEnabled() {
return impl
}

// Two halves of arming the dance, and they sit in different layers on
// purpose: the service owns the state signature (it already holds the JWT
// issuer key the signature is seeded from), the handler owns the transport.
services.Auth.WithDance(
cfg.Auth,
cfg.OauthProviders.Google.ClientSecret,
oauthdance.NewStore(valkeyClient),
googleoauthgw.NewClient(cfg.OauthProviders.Google),
)

return impl.WithOAuthDance(cfg.OauthProviders.Google, cfg.App)
}

func main() {
ctx, stop := signal.NotifyContext(context.Background(),
syscall.SIGTERM,
Expand Down Expand Up @@ -140,13 +175,12 @@ func startAPIServer(
Integrations: integrationapi.New(services.Integration, services.UserSummary),
UserPicker: userpickerapi.New(services.UserPicker),

Auth: apiauth.New(
cfg.Auth,
services.Auth,
services.Token,
services.User,
services.OTP,
),
// The dance dependencies attach only when the feature is
// configured. On an unconfigured instance the routes are never
// registered either, so nothing here is ever read — but wiring a
// gateway holding an empty client secret would be a live object
// waiting for a routing mistake.
Auth: newAuthHandlers(cfg, services, valkeyClient),
Roles: apiroles.New(services.User),
Users: apiusers.New(services.User, services.License),
Invitations: apiinvitations.New(services.Invitation),
Expand All @@ -159,6 +193,7 @@ func startAPIServer(
License: services.License,
},
valkeyClient,
cfg.OAuthDanceEnabled(),
xhttpserver.WithLogger(xecho.NewSlogAdapter(logger)),
)
s.BindRouters(cfg.Environment, meta)
Expand Down
58 changes: 58 additions & 0 deletions deployment/maintmode/dev/app.config.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,33 @@ app:
# Moved in from the auth config — the merged process serves the auth
# routes, so it needs this or those redirects target an empty URL.
frontend_url: http://localhost:3000
# The frontend route the OAuth dance sends the browser back to, appended to
# frontend_url. It belongs to the frontend rather than to this service, so a
# rename there needs no backend release.
#
# Required whenever the dance is armed, with no in-code default: a value from
# config and another from the binary would mean two places to look when a
# redirect lands somewhere unexpected. Leaving it empty leaves the dance
# unregistered rather than redirecting to the frontend's bare origin.
oauth_callback_path: /auth/oauth/callback

# The Path attribute of the two dance cookies, and it must be the EXTERNAL
# prefix the browser sees. Caddy serves this backend under
# `handle_path /auth/*` and STRIPS the prefix, so the app sees
# /api/v1/login/oauth/... while the browser's URL space is
# /auth/api/v1/login/oauth/... A cookie scoped to what the app sees is never
# sent back on the callback, and the dance then dies as a 302 that reads as
# success in every access log.
#
# Keep it the narrowest prefix covering both /start and /callback. Widening to
# "/" also works and only means two httpOnly, minutes-long cookies travel more
# than they need; narrowing past the shared prefix breaks every sign-in.
#
# Required whenever the dance is armed, with no in-code default: leaving it
# empty is not inert — the attribute is then omitted and the browser scopes
# the cookie to the internal request path, which is exactly the value that
# never comes back. So an empty value leaves the dance unregistered instead.
oauth_cookie_path: /auth/api/v1/login/oauth

auth:
# Open self-registration: an unknown, uninvited user signs in as guest. ON in
Expand All @@ -15,6 +42,12 @@ auth:
# email is guessable in principle, and this window plus the attempt ceiling
# below are what bound that.
otp_ttl: 5m
# How long a signed OAuth-dance state stays valid: the span from pressing
# "sign in" to finishing a consent screen, password prompt and second factor
# included. Enforced by the signature rather than by the cookie's MaxAge, so
# lengthening it widens a real window — a captured (state, cookie) pair is
# replayable for exactly this long.
oauth_dance_state_ttl: 10m
# The minimum time BOTH one-time-code endpoints take to answer, whatever they
# did. This is a security knob, not a throttle, and its VALUE is the property:
# an address with no account is refused after a single indexed SELECT, while a
Expand Down Expand Up @@ -133,6 +166,31 @@ oauth_providers:
use_stub: true
google:
client_id: <secret:oauth/google/client_id>
# Backend-driven OAuth dance (RUK-291) — OFF by default.
#
# Uncommenting ALL FOUR keys registers /login/oauth/{provider}/start,
# /callback and /code/exchange. Leaving them commented keeps this instance
# exactly as it was: the BFF path at /login/oauth/exchange/google stays the
# only way in. Setting some but not all logs a warning and registers
# nothing — a dance that cannot reach the provider is worse than no dance.
#
# Before uncommenting client_secret, make sure the key exists in the secret
# store this stand reads: the resolver hard-fails on a missing key, so the
# reference alone stops the instance booting.
#
# redirect_uri must be the EXTERNAL url — Caddy serves this backend under
# `handle_path /auth/*` and strips the prefix, so the app sees /api/v1/...
# while Google must be told /auth/api/v1/... Registering the internal form
# yields a redirect_uri_mismatch that never reaches our logs.
# client_secret: <secret:oauth/google/client_secret>
# redirect_uri: http://localhost:9000/auth/api/v1/login/oauth/google/callback
# auth_url and token_url are the provider's endpoints. They carry no
# in-code default on purpose: an endpoint baked into the binary is one an
# operator cannot see when they need to know where their sign-ins are
# going. The values below are Google's own; a stand pointing at a local
# fake overrides them here.
# auth_url: https://accounts.google.com/o/oauth2/v2/auth
# token_url: https://oauth2.googleapis.com/token
jwtverifier:
jwks_url: https://www.googleapis.com/oauth2/v3/certs
jwt_issuers:
Expand Down
24 changes: 19 additions & 5 deletions deployment/maintmode/dev/app.secrets.sample.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -19,18 +19,32 @@ valkey/password: ""

# Google OAuth — client_id ONLY.
#
# There is deliberately no client_secret: the BFF (maintmode-ui, NextAuth) owns
# the authorization-code exchange with Google. The backend only verifies the
# resulting id_token offline against Google's JWKS, using this client_id as the
# expected audience. Do not add a client_secret here — the code no longer reads
# one, and a second copy of a credential is a second thing to leak.
# Until RUK-291 there was deliberately no client_secret here: the BFF
# (maintmode-ui, NextAuth) owned the authorization-code exchange and the backend
# only verified the resulting id_token offline against Google's JWKS.
#
# RUK-291 made the backend a confidential OAuth client, so a client_secret is now
# meaningful — but ONLY for the backend-driven dance, which is OFF unless
# app.config.yaml also sets oauth_providers.google.client_secret and
# redirect_uri. The BFF path still needs nothing but the client_id.
#
# ORDER MATTERS. The secret resolver hard-fails on a missing key, so a
# <secret:...> reference in app.config.yaml without the matching entry below
# stops the instance booting. Add the key here FIRST, then the config reference.
# Note `make secrets` only copies this sample when app.secrets.yaml is ABSENT —
# on a machine that already has one, add the key by hand or the next run dies at
# config load.
#
# The client_id is not a secret (it ships in every browser redirect), but it
# must match the client the BFF uses or every token fails audience validation.
# Copy it from the Google Cloud console, or from the BFF's
# MAINTMODE_GOOGLE_OAUTH_CLIENT_ID.
"oauth/google/client_id": "CHANGE_ME.apps.googleusercontent.com"

# Backend-driven OAuth dance (RUK-291). Leave as-is unless
# app.config.yaml opts in; the routes stay unregistered without it.
"oauth/google/client_secret": "CHANGE_ME-google-client-secret"

# JWT signing — the merged binary mints and verifies its own tokens.
# issuer_private_key hex-encoded raw P-256 private key
# issuer_kid key id advertised in the in-process JWKS
Expand Down
58 changes: 58 additions & 0 deletions deployment/maintmode/local/app.config.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,33 @@ app:
# Moved in from the auth config — the merged process serves the auth
# routes, so it needs this or those redirects target an empty URL.
frontend_url: http://localhost:9000
# The frontend route the OAuth dance sends the browser back to, appended to
# frontend_url. It belongs to the frontend rather than to this service, so a
# rename there needs no backend release.
#
# Required whenever the dance is armed, with no in-code default: a value from
# config and another from the binary would mean two places to look when a
# redirect lands somewhere unexpected. Leaving it empty leaves the dance
# unregistered rather than redirecting to the frontend's bare origin.
oauth_callback_path: /auth/oauth/callback

# The Path attribute of the two dance cookies, and it must be the EXTERNAL
# prefix the browser sees. Caddy serves this backend under
# `handle_path /auth/*` and STRIPS the prefix, so the app sees
# /api/v1/login/oauth/... while the browser's URL space is
# /auth/api/v1/login/oauth/... A cookie scoped to what the app sees is never
# sent back on the callback, and the dance then dies as a 302 that reads as
# success in every access log.
#
# Keep it the narrowest prefix covering both /start and /callback. Widening to
# "/" also works and only means two httpOnly, minutes-long cookies travel more
# than they need; narrowing past the shared prefix breaks every sign-in.
#
# Required whenever the dance is armed, with no in-code default: leaving it
# empty is not inert — the attribute is then omitted and the browser scopes
# the cookie to the internal request path, which is exactly the value that
# never comes back. So an empty value leaves the dance unregistered instead.
oauth_cookie_path: /auth/api/v1/login/oauth

auth:
# Open self-registration: an unknown, uninvited user signs in as guest. ON in
Expand All @@ -15,6 +42,12 @@ auth:
# email is guessable in principle, and this window plus the attempt ceiling
# below are what bound that.
otp_ttl: 5m
# How long a signed OAuth-dance state stays valid: the span from pressing
# "sign in" to finishing a consent screen, password prompt and second factor
# included. Enforced by the signature rather than by the cookie's MaxAge, so
# lengthening it widens a real window — a captured (state, cookie) pair is
# replayable for exactly this long.
oauth_dance_state_ttl: 10m
# The minimum time BOTH one-time-code endpoints take to answer, whatever they
# did. This is a security knob, not a throttle, and its VALUE is the property:
# an address with no account is refused after a single indexed SELECT, while a
Expand Down Expand Up @@ -133,6 +166,31 @@ oauth_providers:
use_stub: false
google:
client_id: <secret:oauth/google/client_id>
# Backend-driven OAuth dance (RUK-291) — OFF by default.
#
# Uncommenting ALL FOUR keys registers /login/oauth/{provider}/start,
# /callback and /code/exchange. Leaving them commented keeps this instance
# exactly as it was: the BFF path at /login/oauth/exchange/google stays the
# only way in. Setting some but not all logs a warning and registers
# nothing — a dance that cannot reach the provider is worse than no dance.
#
# Before uncommenting client_secret, make sure the key exists in the secret
# store this stand reads: the resolver hard-fails on a missing key, so the
# reference alone stops the instance booting.
#
# redirect_uri must be the EXTERNAL url — Caddy serves this backend under
# `handle_path /auth/*` and strips the prefix, so the app sees /api/v1/...
# while Google must be told /auth/api/v1/... Registering the internal form
# yields a redirect_uri_mismatch that never reaches our logs.
# client_secret: <secret:oauth/google/client_secret>
# redirect_uri: http://localhost:9000/auth/api/v1/login/oauth/google/callback
# auth_url and token_url are the provider's endpoints. They carry no
# in-code default on purpose: an endpoint baked into the binary is one an
# operator cannot see when they need to know where their sign-ins are
# going. The values below are Google's own; a stand pointing at a local
# fake overrides them here.
# auth_url: https://accounts.google.com/o/oauth2/v2/auth
# token_url: https://oauth2.googleapis.com/token
jwtverifier:
jwks_url: https://www.googleapis.com/oauth2/v3/certs
jwt_issuers:
Expand Down
24 changes: 19 additions & 5 deletions deployment/maintmode/local/app.secrets.sample.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -21,18 +21,32 @@

# Google OAuth — client_id ONLY.
#
# There is deliberately no client_secret: the BFF (maintmode-ui, NextAuth) owns
# the authorization-code exchange with Google. The backend only verifies the
# resulting id_token offline against Google's JWKS, using this client_id as the
# expected audience. Do not add a client_secret here — the code no longer reads
# one, and a second copy of a credential is a second thing to leak.
# Until RUK-291 there was deliberately no client_secret here: the BFF
# (maintmode-ui, NextAuth) owned the authorization-code exchange and the backend
# only verified the resulting id_token offline against Google's JWKS.
#
# RUK-291 made the backend a confidential OAuth client, so a client_secret is now
# meaningful — but ONLY for the backend-driven dance, which is OFF unless
# app.config.yaml also sets oauth_providers.google.client_secret and
# redirect_uri. The BFF path still needs nothing but the client_id.
#
# ORDER MATTERS. The secret resolver hard-fails on a missing key, so a
# <secret:...> reference in app.config.yaml without the matching entry below
# stops the instance booting. Add the key here FIRST, then the config reference.
# Note `make secrets` only copies this sample when app.secrets.yaml is ABSENT —
# on a machine that already has one, add the key by hand or the next run dies at
# config load.
#
# The client_id is not a secret (it ships in every browser redirect), but it
# must match the client the BFF uses or every token fails audience validation.
# Copy it from the Google Cloud console, or from the BFF's
# MAINTMODE_GOOGLE_OAUTH_CLIENT_ID.
"oauth/google/client_id": "CHANGE_ME.apps.googleusercontent.com"

# Backend-driven OAuth dance (RUK-291). Leave as-is unless
# app.config.yaml opts in; the routes stay unregistered without it.
"oauth/google/client_secret": "CHANGE_ME-google-client-secret"

# JWT signing — the merged binary mints and verifies its own tokens.
# issuer_private_key hex-encoded raw P-256 private key
# issuer_kid key id advertised in the in-process JWKS
Expand Down
Loading
Loading