diff --git a/.env.example b/.env.example index 81c3f99a..239f06d9 100644 --- a/.env.example +++ b/.env.example @@ -276,14 +276,18 @@ GEODB_PATH=/app/data/GeoLite2-City.mmdb # # Separate from the BOX_GOOGLE_* mailbox client further down. These are for # logging INTO Warmbly; those are for sending FROM a mailbox. +# The redirect URI is served by the API, not the dashboard, and defaults to +# API_PUBLIC_URL/v1/auth//callback. That is the value to register at +# the provider; the backend logs it at boot. # GOOGLE_CLIENT_ID= # GOOGLE_CLIENT_SECRET= -# GOOGLE_REDIRECT_URI=https://app.example.com/auth/google/callback -# GOOGLE_IOS_CLIENT_ID= -# APPLE_APP_ID= +# GOOGLE_REDIRECT_URI= # defaults to API_PUBLIC_URL/v1/auth/google/callback +# GOOGLE_IOS_CLIENT_ID= # the iOS app only; does not enable the browser button +# APPLE_APP_ID= # the Services ID, not the app's bundle id # APPLE_TEAM_ID= # APPLE_KEY_ID= # APPLE_KEY_SECRET= +# APPLE_REDIRECT_URI= # defaults to API_PUBLIC_URL/v1/auth/apple/callback # === Captcha ================================================================== diff --git a/cmd/backend/boot.go b/cmd/backend/boot.go index 33208ec5..bf4adc90 100644 --- a/cmd/backend/boot.go +++ b/cmd/backend/boot.go @@ -85,19 +85,47 @@ func passkeysUsableFor(appURL string) bool { return host == "localhost" || host == "127.0.0.1" || host == "::1" || strings.HasSuffix(host, ".localhost") } -// oidcRedirectURL is where the provider sends the browser back. Explicit -// OIDC_REDIRECT_URL wins; otherwise it derives from the backend's public base, -// which is where the callback handler actually lives. -func oidcRedirectURL() string { - if v := strings.TrimSpace(os.Getenv("OIDC_REDIRECT_URL")); v != "" { +// ssoRedirectURL is where a browser sign-in provider sends the browser back. +// The explicit override wins; otherwise it derives from the backend's public +// base, which is where the callback handler actually lives. +// +// Deriving it matters more than it looks: the callback is served by the API, +// not the dashboard, and pointing it at APP_URL is the mistake that turns a +// correctly registered OAuth client into a sign-in button that 404s. +func ssoRedirectURL(explicit, provider string) string { + if v := strings.TrimSpace(explicit); v != "" { return v } base := strings.TrimRight(os.Getenv("API_PUBLIC_URL"), "/") if base == "" { return "" } - // The route is registered on /v1, not /api/v1: there is no /api prefix. - return base + "/v1/auth/oidc/callback" + // The routes are registered on /v1, not /api/v1: there is no /api prefix. + return base + "/v1/auth/" + provider + "/callback" +} + +func oidcRedirectURL() string { return ssoRedirectURL(os.Getenv("OIDC_REDIRECT_URL"), "oidc") } + +// warnSSORedirectOrigin fires on the one configuration mistake that produces a +// working OAuth client and a broken button: a redirect URI on the dashboard +// origin, which serves no callback route. +func warnSSORedirectOrigin(provider, redirectURL string) { + if redirectURL == "" { + return + } + u, err := url.Parse(redirectURL) + if err != nil { + return + } + if strings.Contains(u.Path, "/v1/auth/") { + return + } + app, aerr := url.Parse(config.AppBaseURL()) + if aerr != nil || app.Host == "" || app.Host != u.Host { + return + } + log.Printf("Warning: the %s sign-in redirect URI %s points at the dashboard, which serves no callback. It should be %s (your API_PUBLIC_URL), registered at the provider.", + provider, redirectURL, ssoRedirectURL("", provider)) } // oauthPublicBaseURL is the base every mailbox-connect redirect_uri is built diff --git a/cmd/backend/envsample b/cmd/backend/envsample index 5d87949b..3a83598b 100644 --- a/cmd/backend/envsample +++ b/cmd/backend/envsample @@ -19,6 +19,7 @@ APPLE_APP_ID="" APPLE_TEAM_ID="" APPLE_KEY_ID="" APPLE_KEY_SECRET="" +APPLE_REDIRECT_URI="" # Config NOTIFY_NAME="Warmbly Notifications" diff --git a/cmd/backend/main.go b/cmd/backend/main.go index 442dcadf..b2fe6fdf 100644 --- a/cmd/backend/main.go +++ b/cmd/backend/main.go @@ -19,7 +19,6 @@ import ( "github.com/getsentry/sentry-go" "github.com/google/uuid" "github.com/meszmate/apple-go" - "github.com/meszmate/google-go" "github.com/warmbly/warmbly/internal/api" "github.com/warmbly/warmbly/internal/api/handler" "github.com/warmbly/warmbly/internal/api/middleware" @@ -79,6 +78,7 @@ import ( "github.com/warmbly/warmbly/internal/app/sequence" "github.com/warmbly/warmbly/internal/app/settings" "github.com/warmbly/warmbly/internal/app/skills" + "github.com/warmbly/warmbly/internal/app/socialauth" "github.com/warmbly/warmbly/internal/app/socket" "github.com/warmbly/warmbly/internal/app/stripe" "github.com/warmbly/warmbly/internal/app/subscription" @@ -230,7 +230,6 @@ func main() { // Deployment auth facts resolved during boot and served by /auth/config. var authPolicy *config.AuthPolicy var passkeysUsable bool - var googleWebSignIn, appleWebSignIn bool var bootstrapService *bootstrap.Service // oidcLogin is nil unless OIDC_ISSUER_URL is configured. var oidcLogin *oidcauth.Service @@ -478,13 +477,6 @@ func main() { log.Fatal(err) } - googleAuth := google.NewAuth( - authCfg.GoogleClientID, - authCfg.GoogleClientSecret, - authCfg.GoogleRedirectURI, - nil, - ) - // Apple Sign in is optional. Skip it entirely when unconfigured (a // self-host without Apple creds); only warn — never fatal — when creds // are present but init fails, so Apple simply stays unavailable. @@ -763,10 +755,6 @@ func main() { captcha, tokenService, emailNotificationService, - &models.ExternalAuth{ - GoogleAuth: googleAuth, - AppleAuth: appleAuthClient, - }, trialService, organizationService, userRepostory, @@ -835,8 +823,38 @@ func main() { log.Printf("Warning: OIDC disabled: %v", oerr) } else { oidcLogin = oidcSvc - authService.WireOIDC(oidcSvc) - log.Printf("OIDC login enabled (issuer %s)", oidcSvc.Issuer()) + authService.WireFederatedProvider(models.IdentityProviderOIDC, oidcSvc) + log.Printf("OIDC login enabled (issuer %s, redirect %s)", oidcSvc.Issuer(), oidcRedirectURL()) + } + } + + // Sign in with Google in the browser. The redirect URI is the one that + // has to be registered at the provider, so it is logged: an operator + // who set the client id and secret and got nothing has no other way to + // find out what Warmbly is asking Google to call back. + if authCfg.GoogleClientID != "" || authCfg.GoogleClientSecret != "" { + redirect := ssoRedirectURL(authCfg.GoogleRedirectURI, "google") + googleLogin, gerr := socialauth.NewGoogle(authCfg.GoogleClientID, authCfg.GoogleClientSecret, redirect) + if gerr != nil { + log.Printf("Warning: Sign in with Google disabled: %v", gerr) + } else { + authService.WireFederatedProvider(models.IdentityProviderGoogle, googleLogin) + warnSSORedirectOrigin("Google", redirect) + log.Printf("Sign in with Google enabled (redirect %s)", googleLogin.RedirectURL()) + } + } + + // Sign in with Apple in the browser, on the same credentials the native + // app path uses. APPLE_APP_ID is the Services ID here. + if appleAuthClient != nil { + redirect := ssoRedirectURL(authCfg.AppleRedirectURI, "apple") + appleLogin, aerr := socialauth.NewApple(appleAuthClient, authCfg.AppleAppID, redirect) + if aerr != nil { + log.Printf("Warning: Sign in with Apple disabled: %v", aerr) + } else { + authService.WireFederatedProvider(models.IdentityProviderApple, appleLogin) + warnSSORedirectOrigin("Apple", redirect) + log.Printf("Sign in with Apple enabled (redirect %s)", appleLogin.RedirectURL()) } } externalAuthProviders = models.ExternalAuthProviders{ @@ -875,8 +893,6 @@ func main() { log.Fatal(passkeyErr) } passkeysUsable = passkeysUsableFor(os.Getenv("APP_URL")) - googleWebSignIn = authCfg.GoogleClientID != "" - appleWebSignIn = authCfg.AppleAppID != "" authCache = cache warnDeploymentURLs(ctx, os.Getenv("APP_URL")) @@ -1640,9 +1656,6 @@ func main() { AuthService: authService, ExternalAuthProviders: externalAuthProviders, - GoogleWebSignIn: googleWebSignIn, - AppleWebSignIn: appleWebSignIn, - OIDCEnabled: oidcLogin != nil, MailDelivers: mailTransport != nil && mailTransport.Delivers, MailTransport: mailTransportKind(mailTransport), MailTransportRef: mailTransport, diff --git a/cmd/backend/sso_redirect_test.go b/cmd/backend/sso_redirect_test.go new file mode 100644 index 00000000..b423f971 --- /dev/null +++ b/cmd/backend/sso_redirect_test.go @@ -0,0 +1,52 @@ +package main + +import "testing" + +// The provider returns the browser to a route the API serves. Deriving it from +// APP_URL instead is the mistake that produces a correctly configured OAuth +// client and a sign-in button that lands on a page that does not exist. +func TestSSORedirectURLDerivesFromAPIPublicURL(t *testing.T) { + t.Setenv("API_PUBLIC_URL", "https://api.example.com/") + + tests := map[string]string{ + "google": "https://api.example.com/v1/auth/google/callback", + "apple": "https://api.example.com/v1/auth/apple/callback", + "oidc": "https://api.example.com/v1/auth/oidc/callback", + } + for provider, want := range tests { + if got := ssoRedirectURL("", provider); got != want { + t.Errorf("ssoRedirectURL(\"\", %q) = %q, want %q", provider, got, want) + } + } +} + +func TestSSORedirectURLPrefersTheExplicitOverride(t *testing.T) { + t.Setenv("API_PUBLIC_URL", "https://api.example.com") + + if got := ssoRedirectURL(" https://proxy.example.com/callback ", "google"); got != "https://proxy.example.com/callback" { + t.Errorf("got %q, want the trimmed override", got) + } +} + +// Without API_PUBLIC_URL there is nothing to derive from, and a relative path +// would be rejected by the provider. Empty is what disables the provider. +func TestSSORedirectURLEmptyWithoutAPublicBase(t *testing.T) { + t.Setenv("API_PUBLIC_URL", "") + t.Setenv("OIDC_REDIRECT_URL", "") + + if got := ssoRedirectURL("", "google"); got != "" { + t.Errorf("got %q, want empty", got) + } + if got := oidcRedirectURL(); got != "" { + t.Errorf("oidcRedirectURL() = %q, want empty", got) + } +} + +func TestOIDCRedirectURLStillReadsItsOwnOverride(t *testing.T) { + t.Setenv("API_PUBLIC_URL", "https://api.example.com") + t.Setenv("OIDC_REDIRECT_URL", "https://sso.example.com/finish") + + if got := oidcRedirectURL(); got != "https://sso.example.com/finish" { + t.Errorf("got %q, want the OIDC_REDIRECT_URL override", got) + } +} diff --git a/deploy/config/env.example b/deploy/config/env.example index bcb8ab2b..390c78ce 100644 --- a/deploy/config/env.example +++ b/deploy/config/env.example @@ -117,7 +117,8 @@ BOX_OUTLOOK_CLIENT_SECRET= # === Social sign-in (all optional; email+password / passkeys work standalone) === # GOOGLE_CLIENT_ID / GOOGLE_CLIENT_SECRET / GOOGLE_REDIRECT_URI are the LOGIN client, -# separate from the BOX_GOOGLE_* mailbox client above. +# separate from the BOX_GOOGLE_* mailbox client above. The redirect URI is served +# by the API and defaults to API_PUBLIC_URL/v1/auth//callback. # GOOGLE_CLIENT_ID= # GOOGLE_CLIENT_SECRET= # GOOGLE_REDIRECT_URI= @@ -126,6 +127,7 @@ BOX_OUTLOOK_CLIENT_SECRET= # APPLE_TEAM_ID= # APPLE_KEY_ID= # APPLE_KEY_SECRET= +# APPLE_REDIRECT_URI= # Passkeys (WebAuthn): derived from APP_URL when unset. Changing the RP ID # invalidates enrolled passkeys, so keep it stable per deployment. # WEBAUTHN_RP_ID=app.example.com diff --git a/docker-compose.yml b/docker-compose.yml index feb4364f..f0c43365 100644 --- a/docker-compose.yml +++ b/docker-compose.yml @@ -327,6 +327,7 @@ services: APPLE_TEAM_ID: ${APPLE_TEAM_ID:-} APPLE_KEY_ID: ${APPLE_KEY_ID:-} APPLE_KEY_SECRET: ${APPLE_KEY_SECRET:-} + APPLE_REDIRECT_URI: ${APPLE_REDIRECT_URI:-} # Passkeys derive from APP_URL when unset. Changing the RP id invalidates # every enrolled passkey, so keep it stable per deployment. WEBAUTHN_RP_ID: ${WEBAUTHN_RP_ID:-} diff --git a/docs/content/docs/api/endpoints.mdx b/docs/content/docs/api/endpoints.mdx index 3c5ba53a..31eff9b8 100644 --- a/docs/content/docs/api/endpoints.mdx +++ b/docs/content/docs/api/endpoints.mdx @@ -279,6 +279,8 @@ These never accept an API key. They depend on a human-bound session: billing flo - `POST /auth/setup` (first-run claim: exchanges the one-time token printed at boot for the owner account. Refused once any account exists) - `GET /auth/providers`, `POST /auth/apple`, `POST /auth/google` (native-app social sign-in) - `POST /auth/oidc/begin`, `GET /auth/oidc/callback` (generic OpenID Connect sign-in) +- `POST /auth/google/begin`, `GET /auth/google/callback`, `POST /auth/apple/begin`, `POST /auth/apple/callback` (browser Sign in with Google and Sign in with Apple) +- `POST /auth/sso/exchange` (swaps the single-use handoff code from any of those callbacks for the session; `POST /auth/oidc/exchange` is the older name for the same endpoint) - `POST /auth/logout`, `POST /auth/logout-all`, `GET /auth/me`, `PATCH /auth/me/onboarding` - `POST /auth/me/avatar`, `DELETE /auth/me/avatar` - `POST /emails/onboarding/oauth/start`, `POST /emails/onboarding/oauth/finish`, `POST /emails/onboarding/smtp-imap` diff --git a/docs/content/docs/api/reference/account-org.mdx b/docs/content/docs/api/reference/account-org.mdx index 10cf412d..b8cb941f 100644 --- a/docs/content/docs/api/reference/account-org.mdx +++ b/docs/content/docs/api/reference/account-org.mdx @@ -259,7 +259,7 @@ Auth: public (not available to API keys). A token pair (same shape as the login confirm success response). -### Social sign-in +### Social sign-in (native apps) All three are public and unavailable to API keys. A provider-verified identity counts as strong auth, so there is no email code step, and a first sign-in creates the account, organization, and trial. @@ -269,7 +269,25 @@ All three are public and unavailable to API keys. A provider-verified identity c | `POST /auth/apple` | `identity_token` (required), `first_name`, `last_name` | Exchanges a Sign in with Apple identity token, verified against Apple's keys. Apple shares the name only with the app, so pass it through for first-sign-in prefill. | | `POST /auth/google` | `id_token` (required) | Exchanges a Google Sign-In ID token, verified against Google's keys and the client ID from `/auth/providers`. | -Apple and Google both return a token pair (same shape as the login confirm success response). +Apple and Google both return a token pair (same shape as the login confirm success response), or a two-factor challenge when the account has TOTP enrolled. + +### Browser sign-in + +A browser cannot authenticate with the provider itself, so it is sent there and comes back with an authorization code. Sign in with Google, Sign in with Apple and generic OpenID Connect are the same three steps against different providers, and which ones a deployment offers is the `providers` array in `GET /auth/config`. + +| Endpoint | Body | Purpose | +|----------|------|---------| +| `POST /auth/google/begin` | none | Returns `{"url": "..."}`, the authorization URL to send the browser to. The state, nonce and PKCE verifier are minted and held server-side, so nothing replayable is handed to the client. | +| `POST /auth/apple/begin` | none | The same for Sign in with Apple. | +| `POST /auth/oidc/begin` | none | The same for the configured OpenID Connect provider. | +| `GET /auth/google/callback` | none | Where Google returns the browser. Answers with a redirect to `/auth/sso?code=...`, or to `/auth/login?sso_error=...` when the sign-in was refused. | +| `POST /auth/apple/callback` | form post | Where Apple returns the browser. Apple uses `response_mode=form_post` because the flow requests the email scope, so this arrives as a form rather than a redirect; it also accepts `GET` for a refusal. | +| `GET /auth/oidc/callback` | none | Where the OpenID Connect provider returns the browser. | +| `POST /auth/sso/exchange` | `code` (required) | Swaps the single-use handoff code from any callback for the session. `POST /auth/oidc/exchange` is the older name for the same endpoint. | + +The callback never returns a token. It hands the browser an opaque, single-use code with a 60 second lifetime and the client exchanges it over `POST`, so no token lands in a URL, browser history, `Referer` header or proxy log. The exchange returns a token pair, or a two-factor challenge when the account has TOTP enrolled. + +Redirect URIs are served by the API, not the dashboard: they default to `/v1/auth//callback` and that is the value to register at the provider. See [deployment](/development/deployment-guide/#sign-in-with-google-or-apple-optional). ## Sessions diff --git a/docs/content/docs/development/accounts-and-access.mdx b/docs/content/docs/development/accounts-and-access.mdx index 236ec011..686c177b 100644 --- a/docs/content/docs/development/accounts-and-access.mdx +++ b/docs/content/docs/development/accounts-and-access.mdx @@ -82,7 +82,7 @@ Older builds returned the invite-only refusal with the text "This server is not | TOTP | No | Applies to every sign-in path, including Google, Apple and single sign-on | | Passkeys | No | Requires a secure context, so `APP_URL` must be HTTPS or a `localhost` address | | Single sign-on (OIDC) | No | The recommended posture when you have no relay. See [single sign-on](#single-sign-on) | -| Sign in with Google or Apple | No | Optional, and unrelated to connecting Gmail mailboxes | +| Sign in with Google or Apple | No | Optional, and unrelated to connecting Gmail mailboxes. The redirect URI is served by the API: see [self-hosting](/development/deployment-guide/#sign-in-with-google-or-apple-optional) | `DISABLE_PASSWORD_LOGIN=true` with no configured provider leaves no way to sign in at all. The `no_sign_in_method` check on [Instance health](/development/instance-health/#no_sign_in_method) fires when that happens. diff --git a/docs/content/docs/development/configuration.mdx b/docs/content/docs/development/configuration.mdx index 90656868..b386a405 100644 --- a/docs/content/docs/development/configuration.mdx +++ b/docs/content/docs/development/configuration.mdx @@ -145,9 +145,11 @@ TRUSTED_PROXIES=10.0.0.0/8,172.16.0.0/12 | `WEBAUTHN_RP_ID` | Passkey relying party id. Derived from `APP_URL` when unset. Changing it invalidates every enrolled passkey | derived | yes | | `WEBAUTHN_RP_ORIGINS` | Origins accepted for passkey ceremonies | derived from `APP_URL` | yes | | `WEBAUTHN_RP_DISPLAY_NAME` | The name the passkey prompt shows | `Warmbly` | yes | -| `GOOGLE_CLIENT_ID`, `GOOGLE_CLIENT_SECRET`, `GOOGLE_REDIRECT_URI` | Sign in with Google. Unrelated to the `BOX_GOOGLE_*` mailbox client | unset | yes | -| `GOOGLE_IOS_CLIENT_ID` | Additional Google client id accepted from the iOS app | unset | yes | -| `APPLE_APP_ID`, `APPLE_TEAM_ID`, `APPLE_KEY_ID`, `APPLE_KEY_SECRET` | Sign in with Apple | unset | yes | +| `GOOGLE_CLIENT_ID`, `GOOGLE_CLIENT_SECRET` | Sign in with Google in the browser. Both are required; unrelated to the `BOX_GOOGLE_*` mailbox client | unset | yes | +| `GOOGLE_REDIRECT_URI` | Redirect URI registered at Google. Served by the API, not the dashboard | `API_PUBLIC_URL` plus `/v1/auth/google/callback` | yes | +| `GOOGLE_IOS_CLIENT_ID` | Additional Google client id accepted from the iOS app. Native only: it does not enable the browser button | unset | yes | +| `APPLE_APP_ID`, `APPLE_TEAM_ID`, `APPLE_KEY_ID`, `APPLE_KEY_SECRET` | Sign in with Apple. `APPLE_APP_ID` is the Services ID | unset | yes | +| `APPLE_REDIRECT_URI` | Return URL registered at Apple. Must be HTTPS | `API_PUBLIC_URL` plus `/v1/auth/apple/callback` | yes | | `APPLE_IOS_BUNDLE_ID` | Bundle id accepted from the iOS app | `com.warmbly.app` | yes | | `OIDC_ISSUER_URL` | Generic OpenID Connect issuer. Discovery runs at boot | unset | yes | | `OIDC_CLIENT_ID`, `OIDC_CLIENT_SECRET` | The client Warmbly authenticates as | unset | yes | diff --git a/docs/content/docs/development/deployment-guide.mdx b/docs/content/docs/development/deployment-guide.mdx index 7d93cfc5..38daf4f2 100644 --- a/docs/content/docs/development/deployment-guide.mdx +++ b/docs/content/docs/development/deployment-guide.mdx @@ -518,16 +518,27 @@ The backend starts the OAuth flow, but **each worker refreshes the token** when Separate from mailboxes, and unrelated to sending. Email and password plus passkeys work without any of this. ```bash -GOOGLE_CLIENT_ID=... # redirect: /auth/google/callback +GOOGLE_CLIENT_ID=... GOOGLE_CLIENT_SECRET=... -GOOGLE_REDIRECT_URI=https://app.example.com/auth/google/callback +# Optional. Defaults to /v1/auth/google/callback +GOOGLE_REDIRECT_URI=https://api.example.com/v1/auth/google/callback -APPLE_APP_ID=... +APPLE_APP_ID=... # the Services ID, not the app's bundle id APPLE_TEAM_ID=... APPLE_KEY_ID=... APPLE_KEY_SECRET=... +# Optional. Defaults to /v1/auth/apple/callback +APPLE_REDIRECT_URI=https://api.example.com/v1/auth/apple/callback ``` + +The provider returns the browser to a route the **backend** serves, which is why the default derives from `API_PUBLIC_URL`. Registering `https://app.example.com/auth/google/callback` at the provider gives you a correctly configured OAuth client and a sign-in button that lands on a dashboard route that does not exist. Warmbly logs the exact URI to register at boot: `Sign in with Google enabled (redirect ...)`. + + +Register that URI as an authorized redirect URI on a **Web application** client in the Google Cloud console, and as a Return URL on the Services ID in the Apple developer console. Apple additionally requires HTTPS, so Sign in with Apple cannot run against a plain-http local install. + +Once either is configured, `GET /auth/config` advertises it and the login screen renders the button. First sign-in provisions the account, its organization and its trial, and answers to `DISABLE_REGISTRATION` and `SSO_AUTO_PROVISION` exactly like a password signup does. + ## Authentication Self-hosted defaults differ from the hosted product, because a deployment you run yourself should not depend on infrastructure you have not set up. `DEPLOYMENT_MODE=self_hosted` picks them; each one is independently overridable. @@ -609,7 +620,7 @@ Three things to know: |---------|-------------| | Stripe billing | `BILLING_PROVIDER=stripe` plus `STRIPE_SECRET_KEY`, `STRIPE_WEBHOOK_SECRET`, and `STRIPE_PUBLISHABLE_KEY`. All three are read at boot and the backend exits if any is missing | | Turnstile captcha | `CAPTCHA_PROVIDER=turnstile` plus `TURNSTILE_SECRET` (backend) and `WARMBLY_TURNSTILE_KEY` (web and admin). Compose pins captcha off, so use a `docker-compose.override.yml` | -| Sign in with Google | `GOOGLE_CLIENT_ID`, `GOOGLE_CLIENT_SECRET`, `GOOGLE_REDIRECT_URI` | +| Sign in with Google | `GOOGLE_CLIENT_ID`, `GOOGLE_CLIENT_SECRET`, `GOOGLE_REDIRECT_URI` (defaults to `/v1/auth/google/callback`) | | Sign in with Apple | `APPLE_APP_ID`, `APPLE_TEAM_ID`, `APPLE_KEY_ID`, `APPLE_KEY_SECRET` | | Mobile push (APNs) | `APNS_KEY_PATH` (or `APNS_KEY`), `APNS_KEY_ID`, `APNS_TEAM_ID`, `APNS_TOPIC` on backend and consumer. Partial config disables push with a warning, never a crash | | Notification tuning | `NOTIFICATION_PUSH_WINDOW` (default `5h`), `NOTIFICATION_EMAIL_DAILY_CAP` (default `25`, `0` means uncapped) | diff --git a/docs/content/docs/development/instance-health.mdx b/docs/content/docs/development/instance-health.mdx index 65747ed8..cbcacd7d 100644 --- a/docs/content/docs/development/instance-health.mdx +++ b/docs/content/docs/development/instance-health.mdx @@ -172,7 +172,15 @@ Always reported, so the mode is never a surprise. `invite_only` means nobody cre ### no_sign_in_method -`DISABLE_PASSWORD_LOGIN=true` with no OIDC, Google or Apple provider configured leaves no way to sign in to this instance at all. Set `DISABLE_PASSWORD_LOGIN=false` or configure `OIDC_ISSUER_URL`. +`DISABLE_PASSWORD_LOGIN=true` with no OIDC, Google or Apple provider configured leaves no way to sign in to this instance at all. Set `DISABLE_PASSWORD_LOGIN=false` or configure `OIDC_ISSUER_URL`. A provider counts as configured only when every value it needs is set, which is what the next two checks report. + +### google_sign_in_incomplete + +Some of the Google sign-in configuration is set and some is missing, so the button is not shown. `GOOGLE_CLIENT_ID` and `GOOGLE_CLIENT_SECRET` are both required, and the callback needs a redirect URI, which derives from `API_PUBLIC_URL` when `GOOGLE_REDIRECT_URI` is unset. The same check warns when the redirect URI points at the dashboard: it is served by the API. See [sign-in methods](/development/accounts-and-access/#sign-in-methods). + +### apple_sign_in_incomplete + +The same for Sign in with Apple, which needs `APPLE_APP_ID` (the Services ID), `APPLE_TEAM_ID`, `APPLE_KEY_ID` and `APPLE_KEY_SECRET` together, plus an HTTPS redirect URI. Apple refuses a plain-http return URL, so Sign in with Apple cannot run against a local http install. ### single_platform_admin diff --git a/go.mod b/go.mod index 393d89e6..26b10d7c 100644 --- a/go.mod +++ b/go.mod @@ -28,7 +28,6 @@ require ( github.com/invopop/jsonschema v0.13.0 github.com/jackc/pgx/v5 v5.9.0 github.com/meszmate/apple-go v0.0.0-20250828163208-7fea48c91b32 - github.com/meszmate/google-go v0.0.0-20251207085436-d08bef99cd5d github.com/microcosm-cc/bluemonday v1.0.27 github.com/mileusna/useragent v1.3.5 github.com/nats-io/nats-server/v2 v2.14.1 diff --git a/go.sum b/go.sum index f67be4fd..992d0bfa 100644 --- a/go.sum +++ b/go.sum @@ -618,8 +618,6 @@ github.com/mattn/go-shellwords v1.0.12 h1:M2zGm7EW6UQJvDeQxo4T51eKPurbeFbe8WtebG github.com/mattn/go-shellwords v1.0.12/go.mod h1:EZzvwXDESEeg03EKmM+RmDnNOPKG4lLtQsUlTZDWQ8Y= github.com/meszmate/apple-go v0.0.0-20250828163208-7fea48c91b32 h1:t39Q4yEIyvvCiUquqFvKwmwJUT7lOYIJhLfrBRalQSM= github.com/meszmate/apple-go v0.0.0-20250828163208-7fea48c91b32/go.mod h1:CbFm63AASrsMV+GeO2/UhxSJJ0oUy52ZGpzynk1OSUI= -github.com/meszmate/google-go v0.0.0-20251207085436-d08bef99cd5d h1:c3KMRRS0X4sMhTYsqzLHvnnh+ZcbHqkfD5wPzE9kftw= -github.com/meszmate/google-go v0.0.0-20251207085436-d08bef99cd5d/go.mod h1:5nuxJ9h3gXhvRJsAhdJuw9uA9ryRvBj9tO/s9Ll1yvo= github.com/mgechev/revive v1.7.0 h1:JyeQ4yO5K8aZhIKf5rec56u0376h8AlKNQEmjfkjKlY= github.com/mgechev/revive v1.7.0/go.mod h1:qZnwcNhoguE58dfi96IJeSTPeZQejNeoMQLUZGi4SW4= github.com/mgutz/ansi v0.0.0-20170206155736-9520e82c474b h1:j7+1HpAFS1zy5+Q4qx1fWh90gTKwiN4QCGoY9TWyyO4= diff --git a/internal/api/handler/auth_config.go b/internal/api/handler/auth_config.go index 88471202..ff0cf340 100644 --- a/internal/api/handler/auth_config.go +++ b/internal/api/handler/auth_config.go @@ -46,6 +46,12 @@ type DeploymentAuthConfig struct { Providers []string `json:"providers"` + // ProviderLabels is what each button should say, keyed by the same + // identifiers. It is how OIDC_PROVIDER_NAME reaches the login screen: a + // deployment behind Authentik should not offer a button labelled with a + // protocol nobody outside this file has heard of. + ProviderLabels map[string]string `json:"provider_labels,omitempty"` + // SelfHosted lets the UI drop hosted-only affordances (billing prompts, // referral fields) that make no sense on someone's own server. SelfHosted bool `json:"self_hosted"` @@ -77,16 +83,11 @@ const accountsDocsURL = "https://docs.warmbly.com/development/accounts-and-acces func (h *Handler) AuthConfig(c *gin.Context) { policy := h.AuthService.Policy() - providers := []string{} - if h.ExternalAuthProviders.GoogleIOSClientID != "" || h.GoogleWebSignIn { - providers = append(providers, "google") - } - if h.ExternalAuthProviders.AppleBundleID != "" || h.AppleWebSignIn { - providers = append(providers, "apple") - } - if h.OIDCEnabled { - providers = append(providers, "oidc") - } + // Only providers this backend can actually complete a browser sign-in + // with. A native iOS client id is not one of them: it advertised a Google + // button the dashboard could not finish, which is the shape of the whole + // defect this list exists to avoid. Native apps read /auth/providers. + providers := h.AuthService.FederatedProviders() registration := h.AuthService.RegistrationMode(c.Request.Context()) @@ -99,6 +100,7 @@ func (h *Handler) AuthConfig(c *gin.Context) { MailDelivers: h.MailDelivers, Passkeys: h.PasskeysUsable, Providers: providers, + ProviderLabels: h.AuthService.FederatedProviderLabels(), SelfHosted: config.SelfHosted(), BillingEnabled: config.BillingProvider() != "none", SetupRequired: h.BootstrapService != nil && h.BootstrapService.Required(c.Request.Context()), diff --git a/internal/api/handler/auth_oidc.go b/internal/api/handler/auth_oidc.go deleted file mode 100644 index 4478cfe2..00000000 --- a/internal/api/handler/auth_oidc.go +++ /dev/null @@ -1,75 +0,0 @@ -package handler - -import ( - "net/http" - "net/url" - - "github.com/gin-gonic/gin" - "github.com/warmbly/warmbly/internal/config" - "github.com/warmbly/warmbly/internal/errx" -) - -// OIDCBegin starts a generic OpenID Connect authorization. -// -// It returns the URL rather than issuing a 302 so the dashboard, which is a -// single-page app on a different origin from the API, can navigate to it -// itself. The state, nonce and PKCE verifier are stored server-side and never -// leave the backend. -func (h *Handler) OIDCBegin(c *gin.Context) { - redirect, err := h.AuthService.OIDCBegin(c.Request.Context()) - if err != nil { - errx.Handle(c, err) - return - } - c.JSON(http.StatusOK, redirect) -} - -// OIDCCallback is where the provider sends the browser back. -// -// It answers with a redirect, not JSON: a person is looking at this response. -// The session itself is held server-side behind a single-use handoff code that -// the dashboard immediately exchanges, so no token ever appears in a URL, -// browser history, Referer header or proxy log. -func (h *Handler) OIDCCallback(c *gin.Context) { - base := config.AppBaseURL() - - // A provider-side refusal arrives as ?error=, not as a failed exchange. - if e := c.Query("error"); e != "" { - c.Redirect(http.StatusFound, base+"/auth/login?sso_error="+url.QueryEscape(e)) - return - } - - handoff, err := h.AuthService.OIDCCallback( - c.Request.Context(), - c.Query("code"), - c.Query("state"), - c.ClientIP(), - c.Request.UserAgent(), - ) - if err != nil { - c.Redirect(http.StatusFound, base+"/auth/login?sso_error="+url.QueryEscape(err.Message)) - return - } - - c.Redirect(http.StatusFound, base+"/auth/sso?code="+url.QueryEscape(handoff)) -} - -type oidcExchangeRequest struct { - Code string `json:"code"` -} - -// OIDCExchange swaps the handoff code for the session. Single use. -func (h *Handler) OIDCExchange(c *gin.Context) { - var req oidcExchangeRequest - if err := c.ShouldBindJSON(&req); err != nil { - errx.Handle(c, errx.ErrInvalid) - return - } - - result, err := h.AuthService.OIDCExchange(c.Request.Context(), req.Code) - if err != nil { - errx.Handle(c, err) - return - } - c.JSON(http.StatusOK, result) -} diff --git a/internal/api/handler/auth_sso.go b/internal/api/handler/auth_sso.go new file mode 100644 index 00000000..6033703f --- /dev/null +++ b/internal/api/handler/auth_sso.go @@ -0,0 +1,153 @@ +package handler + +import ( + "encoding/json" + "net/http" + "net/url" + + "github.com/gin-gonic/gin" + "github.com/warmbly/warmbly/internal/app/auth" + "github.com/warmbly/warmbly/internal/config" + "github.com/warmbly/warmbly/internal/errx" + "github.com/warmbly/warmbly/internal/models" +) + +// Browser sign-in: generic OIDC, Sign in with Google, Sign in with Apple. +// +// All three run the same three steps. Begin returns the provider's +// authorization URL rather than issuing a 302, because the dashboard is a +// single-page app on a different origin from the API and navigates itself. The +// provider then sends the browser to the callback, which answers with a +// redirect (a person is looking at that response) carrying a single-use handoff +// code, and the dashboard exchanges the code for the session over POST. No +// token ever appears in a URL, browser history, Referer header or proxy log. + +// OIDCBegin starts a generic OpenID Connect authorization. +func (h *Handler) OIDCBegin(c *gin.Context) { + h.ssoBegin(c, models.IdentityProviderOIDC) +} + +// GoogleBegin starts a Sign in with Google authorization. +func (h *Handler) GoogleBegin(c *gin.Context) { + h.ssoBegin(c, models.IdentityProviderGoogle) +} + +// AppleBegin starts a Sign in with Apple authorization. +func (h *Handler) AppleBegin(c *gin.Context) { + h.ssoBegin(c, models.IdentityProviderApple) +} + +func (h *Handler) ssoBegin(c *gin.Context, provider string) { + redirect, err := h.AuthService.SSOBegin(c.Request.Context(), provider) + if err != nil { + errx.Handle(c, err) + return + } + c.JSON(http.StatusOK, redirect) +} + +// OIDCCallback is where an OpenID Connect provider sends the browser back. +func (h *Handler) OIDCCallback(c *gin.Context) { + h.ssoCallback(c, auth.SSOCallback{ + Provider: models.IdentityProviderOIDC, + Code: c.Query("code"), + State: c.Query("state"), + }) +} + +// GoogleCallback is where Google sends the browser back. +func (h *Handler) GoogleCallback(c *gin.Context) { + h.ssoCallback(c, auth.SSOCallback{ + Provider: models.IdentityProviderGoogle, + Code: c.Query("code"), + State: c.Query("state"), + }) +} + +// appleCallbackUser is the one-time name payload Apple posts alongside the +// code. Apple shares a person's name exactly once, on their first +// authorization, and never inside the ID token, so a first sign-in that drops +// it leaves an account named after its email local part forever. +type appleCallbackUser struct { + Name struct { + FirstName string `json:"firstName"` + LastName string `json:"lastName"` + } `json:"name"` +} + +// AppleCallback is where Apple sends the browser back. +// +// Requesting any scope forces response_mode=form_post, so this arrives as a +// cross-site POST with a form body rather than a redirect with a query string. +// Nothing here reads a cookie, which is what lets that work: the state is held +// server-side and the form is the only thing that has to survive the hop. +func (h *Handler) AppleCallback(c *gin.Context) { + in := auth.SSOCallback{ + Provider: models.IdentityProviderApple, + Code: formOrQuery(c, "code"), + State: formOrQuery(c, "state"), + } + if raw := formOrQuery(c, "user"); raw != "" { + var u appleCallbackUser + if err := json.Unmarshal([]byte(raw), &u); err == nil { + in.FirstName, in.LastName = u.Name.FirstName, u.Name.LastName + } + } + h.ssoCallback(c, in) +} + +func (h *Handler) ssoCallback(c *gin.Context, in auth.SSOCallback) { + base := config.AppBaseURL() + + // A provider-side refusal arrives as error=, not as a failed exchange. + // Someone who closed the consent screen is not an error worth shouting + // about, so it goes back to the login form quietly. + if e := formOrQuery(c, "error"); e != "" { + if e == "access_denied" || e == "user_cancelled_authorize" { + c.Redirect(http.StatusFound, base+"/auth/login") + return + } + c.Redirect(http.StatusFound, base+"/auth/login?sso_error="+url.QueryEscape(e)) + return + } + + in.IPAddress = c.ClientIP() + in.UserAgent = c.Request.UserAgent() + + handoff, err := h.AuthService.SSOCallbackComplete(c.Request.Context(), in) + if err != nil { + c.Redirect(http.StatusFound, base+"/auth/login?sso_error="+url.QueryEscape(err.Message)) + return + } + + c.Redirect(http.StatusFound, base+"/auth/sso?code="+url.QueryEscape(handoff)) +} + +// formOrQuery reads a parameter from either the posted form or the query +// string, because the same callback serves both response modes. +func formOrQuery(c *gin.Context, key string) string { + if v := c.PostForm(key); v != "" { + return v + } + return c.Query(key) +} + +type ssoExchangeRequest struct { + Code string `json:"code"` +} + +// SSOExchange swaps the handoff code for the session. Single use. +func (h *Handler) SSOExchange(c *gin.Context) { + var req ssoExchangeRequest + if err := c.ShouldBindJSON(&req); err != nil { + errx.Handle(c, errx.ErrInvalid) + return + } + + result, err := h.AuthService.SSOExchange(c.Request.Context(), req.Code) + if err != nil { + errx.Handle(c, err) + return + } + c.JSON(http.StatusOK, result) +} diff --git a/internal/api/handler/handler.go b/internal/api/handler/handler.go index 8d29b13e..09b14ca2 100644 --- a/internal/api/handler/handler.go +++ b/internal/api/handler/handler.go @@ -80,13 +80,12 @@ type Handler struct { ExternalAuthProviders models.ExternalAuthProviders // Deployment facts served by GET /auth/config so the login screen renders - // what this backend can actually do instead of guessing. - GoogleWebSignIn bool - AppleWebSignIn bool - OIDCEnabled bool - MailDelivers bool - PasskeysUsable bool - MailTransport string + // what this backend can actually do instead of guessing. The sign-in + // provider list is not here: it comes from the auth service, which is what + // actually holds the configured flows. + MailDelivers bool + PasskeysUsable bool + MailTransport string // MailTransportRef backs the admin mail diagnostics, which need Preflight // and so cannot go through the EmailNotificationService interface. MailTransportRef *notify.Transport diff --git a/internal/api/routes.go b/internal/api/routes.go index 3584dd04..6f749c3d 100644 --- a/internal/api/routes.go +++ b/internal/api/routes.go @@ -255,12 +255,27 @@ func Run( auth.POST("/apple", h.AppleTokenLogin) auth.POST("/google", h.GoogleTokenLogin) - // Generic OpenID Connect. The only sign-in path with no dependency on - // outbound mail, which is what makes it the one that matters for a - // deployment with no relay. + // Browser sign-in. Generic OIDC is the only sign-in path with no + // dependency on outbound mail, which is what makes it the one that + // matters for a deployment with no relay; Google and Apple are the + // same flow against a fixed provider. Begin hands the SPA the + // authorization URL, the provider returns the browser to the callback, + // and the single-use handoff code is exchanged for the session. + // + // Apple posts its callback (requesting any scope forces + // response_mode=form_post) and can send a refusal as a redirect, so it + // answers on both methods. auth.POST("/oidc/begin", h.OIDCBegin) auth.GET("/oidc/callback", h.OIDCCallback) - auth.POST("/oidc/exchange", h.OIDCExchange) + auth.POST("/google/begin", h.GoogleBegin) + auth.GET("/google/callback", h.GoogleCallback) + auth.POST("/apple/begin", h.AppleBegin) + auth.POST("/apple/callback", h.AppleCallback) + auth.GET("/apple/callback", h.AppleCallback) + auth.POST("/sso/exchange", h.SSOExchange) + // The name the exchange shipped under when OIDC was the only browser + // flow, kept so a client written against it keeps working. + auth.POST("/oidc/exchange", h.SSOExchange) // 2FA login challenge (PUBLIC): exchanges a single-use pending token + // TOTP/recovery code for a real session. Rate-limited in the service diff --git a/internal/app/auth/external_apple.go b/internal/app/auth/external_apple.go deleted file mode 100644 index e043d2ab..00000000 --- a/internal/app/auth/external_apple.go +++ /dev/null @@ -1,40 +0,0 @@ -package auth - -import ( - "context" - "errors" - - "github.com/getsentry/sentry-go" - "github.com/meszmate/apple-go" - "github.com/warmbly/warmbly/internal/app/token" - "github.com/warmbly/warmbly/internal/errx" - "github.com/warmbly/warmbly/internal/models" -) - -func (s *authService) AppleAuth(ctx context.Context, code, ipaddr, userAgent string) (*models.LoginResult, *errx.Error) { - atoken, err := s.externalAuth.AppleAuth.ValidateCode(code) - if err != nil { - if errors.Is(err, apple.ErrorResponseInvalidGrant) { - return nil, errx.ErrExternalCode - } - sentry.CaptureException(err) - return nil, errx.InternalError() - } - - user, err := apple.GetUserInfoFromIDToken(atoken.IDToken) - if err != nil { - return nil, errx.InternalError() - } - - if !user.EmailVerified || user.Email == "" { - return nil, errx.ErrExternalEmail - } - - udb, xerr := s.authRepository.ExternalLogin(ctx, user.Email) - if xerr != nil { - return nil, xerr - } - - // Ban enforcement and the 2FA gate, which this path skipped entirely. - return s.finishLoginAs(ctx, udb.ID, ipaddr, userAgent, token.AuthProviderApple) -} diff --git a/internal/app/auth/external_google.go b/internal/app/auth/external_google.go deleted file mode 100644 index 37ddde9f..00000000 --- a/internal/app/auth/external_google.go +++ /dev/null @@ -1,34 +0,0 @@ -package auth - -import ( - "context" - - "github.com/warmbly/warmbly/internal/app/token" - "github.com/warmbly/warmbly/internal/errx" - "github.com/warmbly/warmbly/internal/models" -) - -func (s *authService) GoogleAuth(ctx context.Context, code, ipaddr, userAgent string) (*models.LoginResult, *errx.Error) { - atoken, err := s.externalAuth.GoogleAuth.Exchange(ctx, code) - if err != nil { - return nil, errx.ErrExternalCode - } - - user, err := s.externalAuth.GoogleAuth.GetUserInfo(ctx, atoken) - if err != nil { - return nil, errx.InternalError() - } - - if !user.EmailVerified || user.Email == "" { - return nil, errx.ErrExternalEmail - } - - udb, xerr := s.authRepository.ExternalLogin(ctx, user.Email) - if xerr != nil { - return nil, xerr - } - - // Ban enforcement and the 2FA gate, which this path skipped entirely: a - // user who enrolled TOTP could sign in without it by choosing Google. - return s.finishLoginAs(ctx, udb.ID, ipaddr, userAgent, token.AuthProviderGoogle) -} diff --git a/internal/app/auth/oidc.go b/internal/app/auth/oidc.go deleted file mode 100644 index 5c877bb7..00000000 --- a/internal/app/auth/oidc.go +++ /dev/null @@ -1,197 +0,0 @@ -package auth - -import ( - "context" - "crypto/rand" - "encoding/hex" - "encoding/json" - "net/mail" - "time" - - "github.com/getsentry/sentry-go" - "github.com/warmbly/warmbly/internal/app/token" - "github.com/warmbly/warmbly/internal/errx" - "github.com/warmbly/warmbly/internal/models" - "github.com/warmbly/warmbly/internal/pkg/idtoken" -) - -// oidcStateTTL bounds how long an in-flight authorization may take. Short -// enough that a leaked state is useless, long enough for a real person to type -// a password and approve an MFA prompt at their IdP. -const oidcStateTTL = 10 * time.Minute - -// OIDCProvider is the generic OpenID Connect client. Satisfied by -// *oidcauth.Service; an interface here so the auth package does not import it -// and tests can stub it. -type OIDCProvider interface { - AuthCodeURL(state, nonce, verifier string) string - Exchange(ctx context.Context, code, verifier, expectedNonce string) (*idtoken.Claims, error) - Issuer() string - ProviderName() string - DefaultOrgID() string -} - -// OIDCRedirect is what the client sends the browser to. -type OIDCRedirect struct { - URL string `json:"url"` -} - -// oidcFlow is the server-side half of one authorization request. Keeping the -// verifier and nonce here, keyed by state, is what makes them single-use: RFC -// 9700 requires PKCE and a one-time state, and an ID token nonce only proves -// anything if the value it is compared against was never reused. -type oidcFlow struct { - Verifier string `json:"verifier"` - Nonce string `json:"nonce"` -} - -func oidcStateKey(state string) string { return "oidc_state:" + state } - -// oidcHandoffTTL is how long the dashboard has to exchange the handoff code for -// the real session. Seconds, not minutes: the redirect and the exchange happen -// back to back. -const oidcHandoffTTL = 60 * time.Second - -func oidcHandoffKey(code string) string { return "oidc_handoff:" + code } - -// mintHandoff stores a completed login behind a single-use code. -// -// The provider redirects a browser to the callback, so that response cannot be -// JSON: it has to be a redirect the user can follow. Putting tokens in the URL -// would leak them into history, Referer and any proxy log, so the redirect -// carries an opaque code and the dashboard exchanges it over POST. -func (s *authService) mintHandoff(ctx context.Context, result *models.LoginResult) (string, *errx.Error) { - code, err := randomHex(32) - if err != nil { - sentry.CaptureException(err) - return "", errx.InternalError() - } - payload, err := json.Marshal(result) - if err != nil { - sentry.CaptureException(err) - return "", errx.InternalError() - } - if err := s.cache.SetEx(ctx, oidcHandoffKey(code), payload, oidcHandoffTTL).Err(); err != nil { - sentry.CaptureException(err) - return "", errx.InternalError() - } - return code, nil -} - -// OIDCExchange swaps a handoff code for the session it stands for. Single use: -// the code is deleted as it is read. -func (s *authService) OIDCExchange(ctx context.Context, code string) (*models.LoginResult, *errx.Error) { - if code == "" { - return nil, errx.ErrToken - } - raw, err := s.cache.GetDel(ctx, oidcHandoffKey(code)).Bytes() - if err != nil || len(raw) == 0 { - return nil, errx.ErrToken - } - var result models.LoginResult - if err := json.Unmarshal(raw, &result); err != nil { - sentry.CaptureException(err) - return nil, errx.InternalError() - } - return &result, nil -} - -// OIDCBegin starts an authorization request. -func (s *authService) OIDCBegin(ctx context.Context) (*OIDCRedirect, *errx.Error) { - if s.oidc == nil { - return nil, errx.ErrExternalProvider - } - - state, err := randomHex(32) - if err != nil { - sentry.CaptureException(err) - return nil, errx.InternalError() - } - nonce, err := randomHex(32) - if err != nil { - sentry.CaptureException(err) - return nil, errx.InternalError() - } - verifier, err := randomHex(32) - if err != nil { - sentry.CaptureException(err) - return nil, errx.InternalError() - } - - payload, err := json.Marshal(oidcFlow{Verifier: verifier, Nonce: nonce}) - if err != nil { - sentry.CaptureException(err) - return nil, errx.InternalError() - } - if err := s.cache.SetEx(ctx, oidcStateKey(state), payload, oidcStateTTL).Err(); err != nil { - sentry.CaptureException(err) - return nil, errx.InternalError() - } - - return &OIDCRedirect{URL: s.oidc.AuthCodeURL(state, nonce, verifier)}, nil -} - -// OIDCCallback completes the authorization and returns a single-use handoff -// code the dashboard exchanges for the session. -func (s *authService) OIDCCallback(ctx context.Context, code, state, ipaddr, userAgent string) (string, *errx.Error) { - if s.oidc == nil { - return "", errx.ErrExternalProvider - } - if code == "" || state == "" { - return "", errx.ErrExternalCode - } - - // Consume the state before doing anything with it. A replayed callback - // finds nothing and is rejected, which is the whole point of one-time - // state. - raw, cerr := s.cache.GetDel(ctx, oidcStateKey(state)).Bytes() - if cerr != nil || len(raw) == 0 { - return "", errx.ErrExternalCode - } - - var flow oidcFlow - if err := json.Unmarshal(raw, &flow); err != nil { - sentry.CaptureException(err) - return "", errx.InternalError() - } - - claims, err := s.oidc.Exchange(ctx, code, flow.Verifier, flow.Nonce) - if err != nil { - // Provider-side failures are user-visible configuration problems more - // often than attacks, so they are worth reporting rather than burying. - sentry.CaptureException(err) - return "", errx.ErrExternalCode - } - - email, perr := mail.ParseAddress(claims.Email) - if perr != nil { - return "", errx.ErrExternalEmail - } - - userID, rerr := s.resolveFederatedUser( - ctx, - models.IdentityProviderOIDC, - claims.Issuer, - claims.Subject, - email, - claims.GivenName, - claims.FamilyName, - ) - if rerr != nil { - return "", rerr - } - - result, lerr := s.finishLoginAs(ctx, userID, ipaddr, userAgent, token.AuthProviderEmail) - if lerr != nil { - return "", lerr - } - return s.mintHandoff(ctx, result) -} - -func randomHex(n int) (string, error) { - buf := make([]byte, n) - if _, err := rand.Read(buf); err != nil { - return "", err - } - return hex.EncodeToString(buf), nil -} diff --git a/internal/app/auth/service.go b/internal/app/auth/service.go index 75b477b6..3a99a95c 100644 --- a/internal/app/auth/service.go +++ b/internal/app/auth/service.go @@ -88,13 +88,18 @@ type AuthService interface { // sign-in resolves accounts by (issuer, subject) instead of email. WireIdentities(r repository.IdentityRepository) - // Generic OIDC. WireOIDC attaches the provider (nil = OIDC disabled). - WireOIDC(p OIDCProvider) - OIDCBegin(ctx context.Context) (*OIDCRedirect, *errx.Error) - // OIDCCallback returns a single-use handoff code, not a session: the + // Browser sign-in: generic OIDC, Sign in with Google, Sign in with Apple. + // WireFederatedProvider attaches one under its identity-provider key + // ("oidc", "google", "apple"); a provider that is never wired stays + // unavailable and its button is never advertised. + WireFederatedProvider(name string, p FederatedProvider) + FederatedProviders() []string + FederatedProviderLabels() map[string]string + SSOBegin(ctx context.Context, provider string) (*SSORedirect, *errx.Error) + // SSOCallbackComplete returns a single-use handoff code, not a session: the // provider redirects a browser here, so the response must be a redirect. - OIDCCallback(ctx context.Context, code, state, ipaddr, userAgent string) (string, *errx.Error) - OIDCExchange(ctx context.Context, code string) (*models.LoginResult, *errx.Error) + SSOCallbackComplete(ctx context.Context, in SSOCallback) (string, *errx.Error) + SSOExchange(ctx context.Context, code string) (*models.LoginResult, *errx.Error) } type authService struct { @@ -107,7 +112,6 @@ type authService struct { emailNotificationService notify.EmailNotificationService cache *cache.Cache captcha *captcha.Turnstile - externalAuth *models.ExternalAuth appleIDTokens IDTokenVerifier googleIDTokens IDTokenVerifier twofa TwoFAChallenger @@ -119,8 +123,9 @@ type authService struct { // unwired repository falls back to the historic email-only matching, which // is only safe for Apple and Google. identities repository.IdentityRepository - // oidc is the generic OpenID Connect provider, nil unless configured. - oidc OIDCProvider + // providers are the configured browser sign-in flows, keyed by identity + // provider ("oidc", "google", "apple"). Empty unless one is configured. + providers map[string]FederatedProvider // policy and mailDelivers govern whether an emailed code gates a login and // whether public signups are open. Defaults are set in NewService so a @@ -142,8 +147,6 @@ func (s *authService) Policy() *config.AuthPolicy { return s.policy } func (s *authService) WireIdentities(r repository.IdentityRepository) { s.identities = r } -func (s *authService) WireOIDC(p OIDCProvider) { s.oidc = p } - func (s *authService) WireReferral(r ReferralAttributor) { s.referral = r } // WireInstanceSettings attaches the instance settings document, so the signup @@ -156,7 +159,6 @@ func NewService( captcha *captcha.Turnstile, tokenService token.TokenService, emailNotificationService notify.EmailNotificationService, - externalAuthData *models.ExternalAuth, trialService trial.TrialService, organizationService organization.OrganizationService, userRepository repository.UserRepository, @@ -168,7 +170,6 @@ func NewService( emailNotificationService: emailNotificationService, cache: cache, captcha: captcha, - externalAuth: externalAuthData, trialService: trialService, organizationService: organizationService, userRepository: userRepository, diff --git a/internal/app/auth/sso.go b/internal/app/auth/sso.go new file mode 100644 index 00000000..9ec82ba0 --- /dev/null +++ b/internal/app/auth/sso.go @@ -0,0 +1,294 @@ +package auth + +import ( + "context" + "crypto/rand" + "encoding/hex" + "encoding/json" + "net/mail" + "time" + + "github.com/getsentry/sentry-go" + "github.com/warmbly/warmbly/internal/app/token" + "github.com/warmbly/warmbly/internal/errx" + "github.com/warmbly/warmbly/internal/models" + "github.com/warmbly/warmbly/internal/pkg/idtoken" +) + +// ssoStateTTL bounds how long an in-flight authorization may take. Short enough +// that a leaked state is useless, long enough for a real person to type a +// password and approve an MFA prompt at their provider. +const ssoStateTTL = 10 * time.Minute + +// FederatedProvider is one browser sign-in flow: generic OIDC, Sign in with +// Google or Sign in with Apple. All three hand back the same verified claims, +// so the service runs one login path for them rather than three. +// +// Satisfied by *oidcauth.Service, *socialauth.Google and *socialauth.Apple; an +// interface here so the auth package imports none of them and tests can stub it. +type FederatedProvider interface { + AuthCodeURL(state, nonce, verifier string) string + Exchange(ctx context.Context, code, verifier, expectedNonce string) (*idtoken.Claims, error) + // ProviderName is the label its button carries. It is what makes + // OIDC_PROVIDER_NAME reach the login screen, so a deployment behind + // Authentik or Keycloak says so instead of "single sign-on". + ProviderName() string +} + +// SSORedirect is what the client sends the browser to. +type SSORedirect struct { + URL string `json:"url"` +} + +// SSOCallback is one provider callback: everything the flow needs that is not +// already held server-side under the state. +// +// FirstName and LastName exist for Apple, which shares the person's name once, +// with the callback, and never inside the ID token. +type SSOCallback struct { + Provider string + Code string + State string + FirstName string + LastName string + IPAddress string + UserAgent string +} + +// ssoFlow is the server-side half of one authorization request. Keeping the +// verifier and nonce here, keyed by state, is what makes them single-use: RFC +// 9700 requires PKCE and a one-time state, and an ID token nonce only proves +// anything if the value it is compared against was never reused. +// +// Provider is stored with them so a state minted for one provider cannot be +// presented at another provider's callback. +type ssoFlow struct { + Provider string `json:"provider"` + Verifier string `json:"verifier"` + Nonce string `json:"nonce"` +} + +func ssoStateKey(state string) string { return "oidc_state:" + state } + +// ssoHandoffTTL is how long the dashboard has to exchange the handoff code for +// the real session. Seconds, not minutes: the redirect and the exchange happen +// back to back. +const ssoHandoffTTL = 60 * time.Second + +func ssoHandoffKey(code string) string { return "oidc_handoff:" + code } + +// WireFederatedProvider attaches one browser sign-in provider under its +// identity-provider key ("oidc", "google", "apple"). A nil provider is ignored, +// so an unconfigured one simply stays unavailable. +func (s *authService) WireFederatedProvider(name string, p FederatedProvider) { + if p == nil || name == "" { + return + } + if s.providers == nil { + s.providers = map[string]FederatedProvider{} + } + s.providers[name] = p +} + +// federatedProviderOrder is the order the login screen renders the buttons in. +var federatedProviderOrder = []string{ + models.IdentityProviderGoogle, + models.IdentityProviderApple, + models.IdentityProviderOIDC, +} + +// FederatedProviders lists the configured providers, so /auth/config can +// advertise exactly the buttons this deployment can actually complete. +func (s *authService) FederatedProviders() []string { + out := make([]string, 0, len(s.providers)) + for _, name := range federatedProviderOrder { + if _, ok := s.providers[name]; ok { + out = append(out, name) + } + } + return out +} + +// FederatedProviderLabels is what each button should say. +func (s *authService) FederatedProviderLabels() map[string]string { + if len(s.providers) == 0 { + return nil + } + out := make(map[string]string, len(s.providers)) + for _, name := range federatedProviderOrder { + if p, ok := s.providers[name]; ok && p.ProviderName() != "" { + out[name] = p.ProviderName() + } + } + return out +} + +// mintHandoff stores a completed login behind a single-use code. +// +// The provider redirects a browser to the callback, so that response cannot be +// JSON: it has to be a redirect the user can follow. Putting tokens in the URL +// would leak them into history, Referer and any proxy log, so the redirect +// carries an opaque code and the dashboard exchanges it over POST. +func (s *authService) mintHandoff(ctx context.Context, result *models.LoginResult) (string, *errx.Error) { + code, err := randomHex(32) + if err != nil { + sentry.CaptureException(err) + return "", errx.InternalError() + } + payload, err := json.Marshal(result) + if err != nil { + sentry.CaptureException(err) + return "", errx.InternalError() + } + if err := s.cache.SetEx(ctx, ssoHandoffKey(code), payload, ssoHandoffTTL).Err(); err != nil { + sentry.CaptureException(err) + return "", errx.InternalError() + } + return code, nil +} + +// SSOExchange swaps a handoff code for the session it stands for. Single use: +// the code is deleted as it is read. +func (s *authService) SSOExchange(ctx context.Context, code string) (*models.LoginResult, *errx.Error) { + if code == "" { + return nil, errx.ErrToken + } + raw, err := s.cache.GetDel(ctx, ssoHandoffKey(code)).Bytes() + if err != nil || len(raw) == 0 { + return nil, errx.ErrToken + } + var result models.LoginResult + if err := json.Unmarshal(raw, &result); err != nil { + sentry.CaptureException(err) + return nil, errx.InternalError() + } + return &result, nil +} + +// SSOBegin starts an authorization request against one provider. +func (s *authService) SSOBegin(ctx context.Context, provider string) (*SSORedirect, *errx.Error) { + p, ok := s.providers[provider] + if !ok || p == nil { + return nil, errx.ErrExternalProvider + } + + state, err := randomHex(32) + if err != nil { + sentry.CaptureException(err) + return nil, errx.InternalError() + } + nonce, err := randomHex(32) + if err != nil { + sentry.CaptureException(err) + return nil, errx.InternalError() + } + verifier, err := randomHex(32) + if err != nil { + sentry.CaptureException(err) + return nil, errx.InternalError() + } + + payload, err := json.Marshal(ssoFlow{Provider: provider, Verifier: verifier, Nonce: nonce}) + if err != nil { + sentry.CaptureException(err) + return nil, errx.InternalError() + } + if err := s.cache.SetEx(ctx, ssoStateKey(state), payload, ssoStateTTL).Err(); err != nil { + sentry.CaptureException(err) + return nil, errx.InternalError() + } + + return &SSORedirect{URL: p.AuthCodeURL(state, nonce, verifier)}, nil +} + +// SSOCallbackComplete finishes an authorization and returns a single-use +// handoff code the dashboard exchanges for the session. +func (s *authService) SSOCallbackComplete(ctx context.Context, in SSOCallback) (string, *errx.Error) { + p, ok := s.providers[in.Provider] + if !ok || p == nil { + return "", errx.ErrExternalProvider + } + if in.Code == "" || in.State == "" { + return "", errx.ErrExternalCode + } + + // Consume the state before doing anything with it. A replayed callback + // finds nothing and is rejected, which is the whole point of one-time + // state. + raw, cerr := s.cache.GetDel(ctx, ssoStateKey(in.State)).Bytes() + if cerr != nil || len(raw) == 0 { + return "", errx.ErrExternalCode + } + + var flow ssoFlow + if err := json.Unmarshal(raw, &flow); err != nil { + sentry.CaptureException(err) + return "", errx.InternalError() + } + // A state is minted for one provider. Presenting it at another provider's + // callback is a mix-up attack (RFC 9700 4.4), not a real sign-in. + if flow.Provider != "" && flow.Provider != in.Provider { + return "", errx.ErrExternalCode + } + + claims, err := p.Exchange(ctx, in.Code, flow.Verifier, flow.Nonce) + if err != nil { + // Provider-side failures are user-visible configuration problems more + // often than attacks, so they are worth reporting rather than burying. + sentry.CaptureException(err) + return "", errx.ErrExternalCode + } + + email, perr := mail.ParseAddress(claims.Email) + if perr != nil { + return "", errx.ErrExternalEmail + } + + firstName, lastName := claims.GivenName, claims.FamilyName + if firstName == "" { + firstName = in.FirstName + } + if lastName == "" { + lastName = in.LastName + } + + userID, rerr := s.resolveFederatedUser( + ctx, + in.Provider, + claims.Issuer, + claims.Subject, + email, + firstName, + lastName, + ) + if rerr != nil { + return "", rerr + } + + result, lerr := s.finishLoginAs(ctx, userID, in.IPAddress, in.UserAgent, sessionProvider(in.Provider)) + if lerr != nil { + return "", lerr + } + return s.mintHandoff(ctx, result) +} + +// sessionProvider is how the session records what the person signed in with, +// shown on the account security page. +func sessionProvider(provider string) string { + switch provider { + case models.IdentityProviderGoogle: + return token.AuthProviderGoogle + case models.IdentityProviderApple: + return token.AuthProviderApple + default: + return token.AuthProviderOIDC + } +} + +func randomHex(n int) (string, error) { + buf := make([]byte, n) + if _, err := rand.Read(buf); err != nil { + return "", err + } + return hex.EncodeToString(buf), nil +} diff --git a/internal/app/auth/sso_test.go b/internal/app/auth/sso_test.go new file mode 100644 index 00000000..e95e795f --- /dev/null +++ b/internal/app/auth/sso_test.go @@ -0,0 +1,71 @@ +package auth + +import ( + "context" + "testing" + + "github.com/warmbly/warmbly/internal/app/token" + "github.com/warmbly/warmbly/internal/models" + "github.com/warmbly/warmbly/internal/pkg/idtoken" +) + +type stubProvider struct{ name string } + +func (s stubProvider) AuthCodeURL(state, nonce, verifier string) string { + return "https://provider/" + state +} +func (s stubProvider) Exchange(ctx context.Context, code, verifier, nonce string) (*idtoken.Claims, error) { + return nil, nil +} +func (s stubProvider) Issuer() string { return "https://" + s.name } +func (s stubProvider) ProviderName() string { return s.name } + +// The login screen renders whatever this returns, so a provider that was never +// wired must never appear: that is exactly how the Google button came to exist +// with no flow behind it. +func TestFederatedProvidersListsOnlyWhatIsWired(t *testing.T) { + s := &authService{} + if got := s.FederatedProviders(); len(got) != 0 { + t.Fatalf("an unconfigured deployment advertised %v", got) + } + + s.WireFederatedProvider(models.IdentityProviderOIDC, stubProvider{name: "oidc"}) + s.WireFederatedProvider(models.IdentityProviderGoogle, stubProvider{name: "google"}) + + got := s.FederatedProviders() + if len(got) != 2 || got[0] != models.IdentityProviderGoogle || got[1] != models.IdentityProviderOIDC { + t.Fatalf("got %v, want google before oidc", got) + } +} + +func TestWireFederatedProviderIgnoresNothing(t *testing.T) { + s := &authService{} + s.WireFederatedProvider(models.IdentityProviderGoogle, nil) + s.WireFederatedProvider("", stubProvider{name: "google"}) + + if got := s.FederatedProviders(); len(got) != 0 { + t.Fatalf("got %v, want nothing wired", got) + } +} + +func TestSSOBeginRefusesAnUnconfiguredProvider(t *testing.T) { + s := &authService{} + if _, err := s.SSOBegin(context.Background(), models.IdentityProviderGoogle); err == nil { + t.Fatal("expected a refusal for a provider this deployment has no client for") + } +} + +// The account security page names what someone signed in with, so a Google +// login must not be recorded as an email one. +func TestSessionProviderNamesTheRealMethod(t *testing.T) { + tests := map[string]string{ + models.IdentityProviderGoogle: token.AuthProviderGoogle, + models.IdentityProviderApple: token.AuthProviderApple, + models.IdentityProviderOIDC: token.AuthProviderOIDC, + } + for provider, want := range tests { + if got := sessionProvider(provider); got != want { + t.Errorf("sessionProvider(%q) = %q, want %q", provider, got, want) + } + } +} diff --git a/internal/app/instancecheck/checks_access.go b/internal/app/instancecheck/checks_access.go index eebd5a24..2e14b7bf 100644 --- a/internal/app/instancecheck/checks_access.go +++ b/internal/app/instancecheck/checks_access.go @@ -3,6 +3,8 @@ package instancecheck import ( "context" "fmt" + "net/url" + "strings" ) const ( @@ -21,6 +23,8 @@ func accessChecks() []check { return []check{ {id: "registration_mode", run: checkRegistrationMode}, {id: "no_sign_in_method", run: checkNoSignInMethod}, + {id: "google_sign_in_incomplete", run: checkGoogleSignInIncomplete}, + {id: "apple_sign_in_incomplete", run: checkAppleSignInIncomplete}, {id: "single_platform_admin", run: checkSinglePlatformAdmin}, {id: "bootstrap_password_still_set", run: checkBootstrapPasswordStillSet}, {id: "setup_link_outstanding", run: checkSetupLinkOutstanding}, @@ -43,7 +47,7 @@ func checkNoSignInMethod(ctx context.Context, d Deps, in Input) *Finding { if env("OIDC_ISSUER_URL") != "" || runtimeOf(d).OIDCConfigured { return nil } - if env("GOOGLE_CLIENT_ID") != "" || env("APPLE_APP_ID") != "" { + if googleSignInConfigured() || appleSignInConfigured() { return nil } return result(CategoryAccess, SeverityError, "No way to sign in", @@ -52,6 +56,122 @@ func checkNoSignInMethod(ctx context.Context, d Deps, in Input) *Finding { docsSignIn) } +// googleSignInConfigured and appleSignInConfigured mirror what the backend +// actually requires to wire the provider. A client id on its own enables +// nothing, which is why "I set GOOGLE_CLIENT_ID and the button does not work" +// is the report this pair of checks exists to answer. +func googleSignInConfigured() bool { + return env("GOOGLE_CLIENT_ID") != "" && env("GOOGLE_CLIENT_SECRET") != "" && ssoRedirectConfigured("GOOGLE_REDIRECT_URI") +} + +func appleSignInConfigured() bool { + return env("APPLE_APP_ID") != "" && env("APPLE_TEAM_ID") != "" && env("APPLE_KEY_ID") != "" && + env("APPLE_KEY_SECRET") != "" && ssoRedirectConfigured("APPLE_REDIRECT_URI") +} + +func ssoRedirectConfigured(key string) bool { + return env(key) != "" || env("API_PUBLIC_URL") != "" +} + +func checkGoogleSignInIncomplete(ctx context.Context, d Deps, in Input) *Finding { + if env("GOOGLE_CLIENT_ID") == "" && env("GOOGLE_CLIENT_SECRET") == "" { + return nil + } + if missing := firstUnset("GOOGLE_CLIENT_ID", "GOOGLE_CLIENT_SECRET"); missing != "" { + return result(CategoryAccess, SeverityError, "Sign in with Google is half configured", + fmt.Sprintf("%s is not set, so the Google button is not shown and the sign-in cannot complete. "+ + "Both halves of the OAuth client are required.", missing), + docsSignIn) + } + if !ssoRedirectConfigured("GOOGLE_REDIRECT_URI") { + return result(CategoryAccess, SeverityError, "Sign in with Google has no redirect URL", + "The Google client is configured but there is no redirect URI: API_PUBLIC_URL is empty and GOOGLE_REDIRECT_URI is not set, "+ + "so Sign in with Google is disabled. Set API_PUBLIC_URL to this backend's public base.", + docsSignIn) + } + return misdirectedSSORedirect("Google", "GOOGLE_REDIRECT_URI", "google") +} + +func checkAppleSignInIncomplete(ctx context.Context, d Deps, in Input) *Finding { + keys := []string{"APPLE_APP_ID", "APPLE_TEAM_ID", "APPLE_KEY_ID", "APPLE_KEY_SECRET"} + if allUnset(keys...) { + return nil + } + if missing := firstUnset(keys...); missing != "" { + return result(CategoryAccess, SeverityError, "Sign in with Apple is half configured", + fmt.Sprintf("%s is not set, so the Apple button is not shown. Sign in with Apple needs the Services ID, "+ + "the team id, and the key id and key together.", missing), + docsSignIn) + } + if !ssoRedirectConfigured("APPLE_REDIRECT_URI") { + return result(CategoryAccess, SeverityError, "Sign in with Apple has no redirect URL", + "The Apple credentials are set but there is no redirect URI: API_PUBLIC_URL is empty and APPLE_REDIRECT_URI is not set, "+ + "so Sign in with Apple is disabled. Set API_PUBLIC_URL to this backend's public base.", + docsSignIn) + } + if redirect := ssoRedirectURI("APPLE_REDIRECT_URI", "apple"); !strings.HasPrefix(redirect, "https://") { + return result(CategoryAccess, SeverityError, "Sign in with Apple needs an HTTPS redirect URL", + fmt.Sprintf("Apple refuses a plain-http return URL, so Sign in with Apple is disabled with the redirect URI at %s. "+ + "Put this backend behind HTTPS.", redirect), + docsSignIn) + } + return misdirectedSSORedirect("Apple", "APPLE_REDIRECT_URI", "apple") +} + +// misdirectedSSORedirect catches the mistake that produces a valid OAuth client +// and a broken button: a redirect URI on the dashboard origin. The callback is +// served by the API, so the provider lands the browser on a dashboard route +// that does not exist. +func misdirectedSSORedirect(provider, key, slug string) *Finding { + raw := env(key) + if raw == "" { + return nil + } + u, err := url.Parse(raw) + if err != nil || u.Host == "" || strings.Contains(u.Path, "/v1/auth/") { + return nil + } + if !strings.EqualFold(u.Host, hostOf(appURL())) { + return nil + } + return result(CategoryAccess, SeverityWarning, fmt.Sprintf("The %s redirect URI points at the dashboard", provider), + fmt.Sprintf("%s is %s, which is the dashboard. The callback is served by the API, so the sign-in ends on a page that does not exist. "+ + "Use %s and register that at the provider.", key, raw, ssoRedirectURI("", slug)), + docsSignIn) +} + +// ssoRedirectURI mirrors the backend's derivation so a finding can name the +// exact value to register at the provider. +func ssoRedirectURI(key, slug string) string { + if key != "" { + if v := env(key); v != "" { + return v + } + } + if base := strings.TrimRight(env("API_PUBLIC_URL"), "/"); base != "" { + return base + "/v1/auth/" + slug + "/callback" + } + return "" +} + +func allUnset(keys ...string) bool { + for _, key := range keys { + if env(key) != "" { + return false + } + } + return true +} + +func firstUnset(keys ...string) string { + for _, key := range keys { + if env(key) == "" { + return key + } + } + return "" +} + func checkSinglePlatformAdmin(ctx context.Context, d Deps, in Input) *Finding { if d.DB == nil { return nil diff --git a/internal/app/instancecheck/checks_signin_test.go b/internal/app/instancecheck/checks_signin_test.go new file mode 100644 index 00000000..46689d95 --- /dev/null +++ b/internal/app/instancecheck/checks_signin_test.go @@ -0,0 +1,109 @@ +package instancecheck + +import ( + "context" + "strings" + "testing" +) + +func clearSignInEnv(t *testing.T) { + t.Helper() + for _, key := range []string{ + "GOOGLE_CLIENT_ID", "GOOGLE_CLIENT_SECRET", "GOOGLE_REDIRECT_URI", + "APPLE_APP_ID", "APPLE_TEAM_ID", "APPLE_KEY_ID", "APPLE_KEY_SECRET", "APPLE_REDIRECT_URI", + "API_PUBLIC_URL", "APP_URL", + } { + t.Setenv(key, "") + } +} + +func TestGoogleSignInSilentWhenNothingIsConfigured(t *testing.T) { + clearSignInEnv(t) + + if f := checkGoogleSignInIncomplete(context.Background(), Deps{}, Input{}); f != nil { + t.Fatalf("reported %q on a deployment that never asked for Google sign-in", f.Title) + } +} + +// The report behind this check: a client id was set, nothing worked, and +// nothing said why. +func TestGoogleSignInReportsAMissingSecret(t *testing.T) { + clearSignInEnv(t) + t.Setenv("GOOGLE_CLIENT_ID", "client-id") + t.Setenv("API_PUBLIC_URL", "https://api.example.com") + + f := checkGoogleSignInIncomplete(context.Background(), Deps{}, Input{}) + if f == nil { + t.Fatal("a client id with no secret was reported as healthy") + } + if !strings.Contains(f.Message, "GOOGLE_CLIENT_SECRET") { + t.Errorf("the finding does not name the missing variable: %s", f.Message) + } +} + +func TestGoogleSignInReportsAMissingRedirectBase(t *testing.T) { + clearSignInEnv(t) + t.Setenv("GOOGLE_CLIENT_ID", "client-id") + t.Setenv("GOOGLE_CLIENT_SECRET", "client-secret") + + f := checkGoogleSignInIncomplete(context.Background(), Deps{}, Input{}) + if f == nil { + t.Fatal("a complete client with nowhere to call back was reported as healthy") + } + if !strings.Contains(f.Message, "API_PUBLIC_URL") { + t.Errorf("the finding does not name what to set: %s", f.Message) + } +} + +func TestGoogleSignInWarnsOnADashboardRedirect(t *testing.T) { + clearSignInEnv(t) + t.Setenv("GOOGLE_CLIENT_ID", "client-id") + t.Setenv("GOOGLE_CLIENT_SECRET", "client-secret") + t.Setenv("APP_URL", "https://app.example.com") + t.Setenv("API_PUBLIC_URL", "https://api.example.com") + t.Setenv("GOOGLE_REDIRECT_URI", "https://app.example.com/auth/google/callback") + + f := checkGoogleSignInIncomplete(context.Background(), Deps{}, Input{}) + if f == nil { + t.Fatal("a redirect URI pointing at the dashboard was reported as healthy") + } + if !strings.Contains(f.Message, "https://api.example.com/v1/auth/google/callback") { + t.Errorf("the finding does not name the URI to register instead: %s", f.Message) + } +} + +func TestGoogleSignInSilentWhenFullyConfigured(t *testing.T) { + clearSignInEnv(t) + t.Setenv("GOOGLE_CLIENT_ID", "client-id") + t.Setenv("GOOGLE_CLIENT_SECRET", "client-secret") + t.Setenv("APP_URL", "https://app.example.com") + t.Setenv("API_PUBLIC_URL", "https://api.example.com") + + if f := checkGoogleSignInIncomplete(context.Background(), Deps{}, Input{}); f != nil { + t.Fatalf("reported %q on a correctly configured deployment: %s", f.Title, f.Message) + } +} + +func TestAppleSignInNeedsEveryHalfAndHTTPS(t *testing.T) { + clearSignInEnv(t) + t.Setenv("APPLE_APP_ID", "com.example.service") + t.Setenv("API_PUBLIC_URL", "https://api.example.com") + + f := checkAppleSignInIncomplete(context.Background(), Deps{}, Input{}) + if f == nil || !strings.Contains(f.Message, "APPLE_TEAM_ID") { + t.Fatalf("a Services ID on its own was not reported as incomplete: %+v", f) + } + + t.Setenv("APPLE_TEAM_ID", "TEAM") + t.Setenv("APPLE_KEY_ID", "KEY") + t.Setenv("APPLE_KEY_SECRET", "secret") + if f := checkAppleSignInIncomplete(context.Background(), Deps{}, Input{}); f != nil { + t.Fatalf("reported %q on a correctly configured deployment: %s", f.Title, f.Message) + } + + t.Setenv("API_PUBLIC_URL", "http://localhost:8080") + f = checkAppleSignInIncomplete(context.Background(), Deps{}, Input{}) + if f == nil || !strings.Contains(f.Title, "HTTPS") { + t.Fatalf("a plain-http return URL was accepted: %+v", f) + } +} diff --git a/internal/app/instanceconfig/entries.go b/internal/app/instanceconfig/entries.go index 8e3e4e56..0f6df703 100644 --- a/internal/app/instanceconfig/entries.go +++ b/internal/app/instanceconfig/entries.go @@ -449,16 +449,22 @@ var table = []Entry{ }, { Key: "GOOGLE_REDIRECT_URI", Group: GroupAuth, RuntimeChangeable: ChangeBootOnly, - Effect: "Where Google sends the browser back after sign-in.", - DocsAnchor: docsSignIn, - Resolve: envValue("GOOGLE_REDIRECT_URI"), + Effect: "Where Google sends the browser back after sign-in. Served by the API, not the dashboard; derived from API_PUBLIC_URL when unset. This is the URI to register at the provider.", + DocsAnchor: docsSignIn, WhenUnset: SourceDerived, + Resolve: ssoRedirect("GOOGLE_REDIRECT_URI", "google"), }, { Key: "APPLE_APP_ID", Group: GroupAuth, RuntimeChangeable: ChangeBootOnly, - Effect: "The Sign in with Apple service identifier.", + Effect: "The Sign in with Apple service identifier (the Services ID, not the app's bundle id).", DocsAnchor: docsSignIn, Resolve: envValue("APPLE_APP_ID"), }, + { + Key: "APPLE_REDIRECT_URI", Group: GroupAuth, RuntimeChangeable: ChangeBootOnly, + Effect: "Where Apple sends the browser back after sign-in. Must be https; derived from API_PUBLIC_URL when unset.", + DocsAnchor: docsSignIn, WhenUnset: SourceDerived, + Resolve: ssoRedirect("APPLE_REDIRECT_URI", "apple"), + }, { Key: "APPLE_TEAM_ID", Group: GroupAuth, RuntimeChangeable: ChangeBootOnly, Effect: "The Apple developer team the key belongs to.", @@ -751,6 +757,21 @@ var table = []Entry{ }, } +// ssoRedirect mirrors the backend's own derivation, so the configuration page +// shows the URI that has to be registered at the provider rather than an empty +// cell whenever the operator left the override unset. +func ssoRedirect(key, provider string) func(*Runtime) string { + return func(*Runtime) string { + if v := trimmed(key); v != "" { + return v + } + if base := strings.TrimRight(trimmed("API_PUBLIC_URL"), "/"); base != "" { + return base + "/v1/auth/" + provider + "/callback" + } + return "" + } +} + func envValue(key string) func(*Runtime) string { return func(*Runtime) string { return os.Getenv(key) } } diff --git a/internal/app/socialauth/socialauth.go b/internal/app/socialauth/socialauth.go new file mode 100644 index 00000000..d7b4eabc --- /dev/null +++ b/internal/app/socialauth/socialauth.go @@ -0,0 +1,213 @@ +// Package socialauth is browser Sign in with Google and Sign in with Apple. +// +// The native apps authenticate with the provider on device and hand the +// backend a signed ID token, which internal/pkg/idtoken verifies and the auth +// service turns into a session. A browser cannot do that: it has to be sent to +// the provider and come back with an authorization code, and until now nothing +// in the backend owned that half. GOOGLE_CLIENT_ID was read at boot, the login +// screen rendered a Google button because of it, and the button led to a URL no +// route served. +// +// Both providers here end where generic OIDC ends, at a verified +// idtoken.Claims, so the auth service runs one federated login for all three: +// the same (issuer, subject) identity keying, the same just-in-time +// provisioning, the same ban and 2FA gates. +package socialauth + +import ( + "context" + "errors" + "fmt" + "strings" + + apple "github.com/meszmate/apple-go" + "github.com/warmbly/warmbly/internal/pkg/idtoken" + "golang.org/x/oauth2" + googleendpoint "golang.org/x/oauth2/google" +) + +// Google is the browser authorization-code flow for Sign in with Google. +type Google struct { + oauth *oauth2.Config + verifier *idtoken.Verifier +} + +// NewGoogle builds the flow. Both halves of the client credential and a +// redirect URL are required: Google refuses the exchange without the secret and +// rejects the authorization request outright without a registered redirect, and +// either mistake would otherwise only surface as a failed sign-in. +func NewGoogle(clientID, clientSecret, redirectURL string) (*Google, error) { + if clientID == "" || clientSecret == "" { + return nil, errors.New("socialauth: GOOGLE_CLIENT_ID and GOOGLE_CLIENT_SECRET are both required") + } + if redirectURL == "" { + return nil, errors.New("socialauth: GOOGLE_REDIRECT_URI is required (or set API_PUBLIC_URL and let it derive)") + } + if !strings.HasPrefix(redirectURL, "http://") && !strings.HasPrefix(redirectURL, "https://") { + return nil, fmt.Errorf("socialauth: GOOGLE_REDIRECT_URI %q is not an absolute URL", redirectURL) + } + + return &Google{ + oauth: &oauth2.Config{ + ClientID: clientID, + ClientSecret: clientSecret, + RedirectURL: redirectURL, + // The ID token carries everything a login needs, so no userinfo + // call and no token to store: nothing here grants access to + // anything in the person's Google account. + Scopes: []string{"openid", "email", "profile"}, + Endpoint: googleendpoint.Endpoint, + }, + verifier: idtoken.GoogleVerifier(clientID), + }, nil +} + +func (g *Google) ProviderName() string { return "Google" } + +// RedirectURL is the URI that has to be registered at the provider, surfaced so +// boot can log it: an operator who configured the client and got nothing has no +// other way to see what Warmbly is asking the provider to call back. +func (g *Google) RedirectURL() string { return g.oauth.RedirectURL } + +// AuthCodeURL builds the authorization request. PKCE is mandatory for every +// client type under RFC 9700, including confidential ones, and the nonce is +// what binds the returned ID token to this attempt. +// +// prompt=select_account is deliberate: without it Google silently reuses +// whichever account the browser is already signed into, which on a shared +// machine signs the wrong person in with no way to notice. +func (g *Google) AuthCodeURL(state, nonce, verifier string) string { + return g.oauth.AuthCodeURL( + state, + oauth2.S256ChallengeOption(verifier), + oauth2.SetAuthURLParam("nonce", nonce), + oauth2.SetAuthURLParam("prompt", "select_account"), + ) +} + +// Exchange trades the authorization code for a verified identity. +func (g *Google) Exchange(ctx context.Context, code, verifier, expectedNonce string) (*idtoken.Claims, error) { + tok, err := g.oauth.Exchange(ctx, code, oauth2.VerifierOption(verifier)) + if err != nil { + return nil, fmt.Errorf("socialauth: google code exchange: %w", err) + } + + rawID, ok := tok.Extra("id_token").(string) + if !ok || rawID == "" { + return nil, errors.New("socialauth: google token response carried no id_token") + } + + claims, err := g.verifier.Verify(ctx, rawID) + if err != nil { + return nil, fmt.Errorf("socialauth: verifying google id_token: %w", err) + } + if err := checkIdentity(claims, expectedNonce); err != nil { + return nil, err + } + // The verifier already collapses Google's two issuer spellings onto the + // https form, which is what the identity is keyed on. + return claims, nil +} + +// Apple is the browser authorization-code flow for Sign in with Apple. +// +// Apple only puts the email claim in the ID token when the email scope is +// requested, and requiring a scope forces response_mode=form_post, so its +// callback arrives as a cross-site POST rather than a redirect with a query +// string. That is the one structural difference from Google here. +type Apple struct { + client apple.AppleAuth + servicesID string + redirectURL string + verifier *idtoken.Verifier +} + +// NewApple builds the flow from the same credentials the native path uses. The +// client id is the Services ID (the web identifier), not the app's bundle ID. +func NewApple(client apple.AppleAuth, servicesID, redirectURL string) (*Apple, error) { + if client == nil { + return nil, errors.New("socialauth: apple client is not configured") + } + if servicesID == "" { + return nil, errors.New("socialauth: APPLE_APP_ID is required") + } + if redirectURL == "" { + return nil, errors.New("socialauth: APPLE_REDIRECT_URI is required (or set API_PUBLIC_URL and let it derive)") + } + // Apple rejects a plain-http redirect URI outright, so a deployment that + // would never work is better refused at boot than at the first sign-in. + if !strings.HasPrefix(redirectURL, "https://") { + return nil, fmt.Errorf("socialauth: Apple requires an https redirect URI, got %q", redirectURL) + } + + return &Apple{ + client: client, + servicesID: servicesID, + redirectURL: redirectURL, + verifier: idtoken.AppleVerifier(servicesID), + }, nil +} + +func (a *Apple) ProviderName() string { return "Apple" } +func (a *Apple) RedirectURL() string { return a.redirectURL } + +// AuthCodeURL builds the authorization request. Apple does not support PKCE on +// the web flow, so the verifier is ignored; one-time state and the nonce inside +// the ID token are what bind the response to this attempt. +func (a *Apple) AuthCodeURL(state, nonce, _ string) string { + return apple.AuthorizeURL(apple.AuthorizeURLConfig{ + ClientID: a.servicesID, + RedirectURI: a.redirectURL, + State: state, + Nonce: nonce, + Scope: []string{"name", "email"}, + ResponseType: apple.ResponseTypeCode, + ResponseMode: apple.ResponseModeFormPost, + }) +} + +// Exchange trades the authorization code for a verified identity. The ID token +// comes straight off Apple's token endpoint, but it is still verified against +// Apple's published keys: a token nobody checked is a token nobody can trust, +// and the audience check is what stops one issued for a different client. +func (a *Apple) Exchange(ctx context.Context, code, _, expectedNonce string) (*idtoken.Claims, error) { + resp, err := a.client.ValidateCodeWithRedirectURI(code, a.redirectURL) + if err != nil { + if errors.Is(err, apple.ErrorResponseInvalidGrant) { + return nil, fmt.Errorf("socialauth: apple rejected the authorization code: %w", err) + } + return nil, fmt.Errorf("socialauth: apple code exchange: %w", err) + } + if resp == nil || resp.IDToken == "" { + return nil, errors.New("socialauth: apple token response carried no id_token") + } + + claims, err := a.verifier.Verify(ctx, resp.IDToken) + if err != nil { + return nil, fmt.Errorf("socialauth: verifying apple id_token: %w", err) + } + if err := checkIdentity(claims, expectedNonce); err != nil { + return nil, err + } + return claims, nil +} + +// checkIdentity is the part neither provider does for us: the nonce proves the +// token was minted for this authorization request rather than replayed from +// another one, and an unverified address is how federated login turns into +// account takeover at any issuer where addresses are self-asserted. +func checkIdentity(claims *idtoken.Claims, expectedNonce string) error { + if expectedNonce == "" || claims.Nonce != expectedNonce { + return errors.New("socialauth: id_token nonce does not match this authorization request") + } + if claims.Subject == "" { + return errors.New("socialauth: id_token carried no subject") + } + if claims.Email == "" { + return errors.New("socialauth: id_token carried no email claim") + } + if !claims.EmailVerified { + return errors.New("socialauth: the provider did not report this email address as verified") + } + return nil +} diff --git a/internal/app/socialauth/socialauth_test.go b/internal/app/socialauth/socialauth_test.go new file mode 100644 index 00000000..235f54db --- /dev/null +++ b/internal/app/socialauth/socialauth_test.go @@ -0,0 +1,152 @@ +package socialauth + +import ( + "net/url" + "strings" + "testing" + + "github.com/warmbly/warmbly/internal/pkg/idtoken" +) + +func TestNewGoogleRequiresBothHalvesOfTheClient(t *testing.T) { + tests := []struct { + name string + clientID, clientSecret, redirectURL string + }{ + {"no id", "", "secret", "https://api.example.com/v1/auth/google/callback"}, + {"no secret", "id", "", "https://api.example.com/v1/auth/google/callback"}, + {"no redirect", "id", "secret", ""}, + {"relative redirect", "id", "secret", "/v1/auth/google/callback"}, + } + + for _, tt := range tests { + if _, err := NewGoogle(tt.clientID, tt.clientSecret, tt.redirectURL); err == nil { + t.Errorf("%s: expected an error, got a usable provider", tt.name) + } + } +} + +// The authorization request is the half an operator cannot inspect, so every +// parameter the flow depends on later is asserted here: without the challenge +// the exchange is refused, and without the nonce the returned ID token cannot +// be bound to this attempt. +func TestGoogleAuthCodeURLCarriesPKCEAndNonce(t *testing.T) { + g, err := NewGoogle("client-id", "client-secret", "https://api.example.com/v1/auth/google/callback") + if err != nil { + t.Fatal(err) + } + + raw := g.AuthCodeURL("state-value", "nonce-value", "verifier-value") + u, err := url.Parse(raw) + if err != nil { + t.Fatal(err) + } + q := u.Query() + + if got := q.Get("client_id"); got != "client-id" { + t.Errorf("client_id = %q", got) + } + if got := q.Get("redirect_uri"); got != "https://api.example.com/v1/auth/google/callback" { + t.Errorf("redirect_uri = %q", got) + } + if got := q.Get("state"); got != "state-value" { + t.Errorf("state = %q", got) + } + if got := q.Get("nonce"); got != "nonce-value" { + t.Errorf("nonce = %q", got) + } + if got := q.Get("code_challenge_method"); got != "S256" { + t.Errorf("code_challenge_method = %q", got) + } + // The challenge is the hash of the verifier, never the verifier itself. + if got := q.Get("code_challenge"); got == "" || got == "verifier-value" { + t.Errorf("code_challenge = %q, want the S256 hash of the verifier", got) + } + if got := q.Get("prompt"); got != "select_account" { + t.Errorf("prompt = %q, want select_account so a shared browser does not silently reuse an account", got) + } + if scope := q.Get("scope"); !strings.Contains(scope, "openid") || !strings.Contains(scope, "email") { + t.Errorf("scope = %q, want openid and email", scope) + } +} + +func TestNewAppleRefusesPlainHTTPRedirect(t *testing.T) { + if _, err := NewApple(stubAppleClient{}, "com.example.service", "http://localhost:8080/v1/auth/apple/callback"); err == nil { + t.Fatal("expected an error: Apple refuses a plain-http return URL") + } + if _, err := NewApple(nil, "com.example.service", "https://api.example.com/v1/auth/apple/callback"); err == nil { + t.Fatal("expected an error when the Apple client is not configured") + } + if _, err := NewApple(stubAppleClient{}, "", "https://api.example.com/v1/auth/apple/callback"); err == nil { + t.Fatal("expected an error when the Services ID is missing") + } +} + +// Apple only puts the email claim in the ID token when the email scope is +// requested, and any scope forces form_post. Losing either turns first sign-in +// into an account with no address. +func TestAppleAuthCodeURLRequestsEmailByFormPost(t *testing.T) { + a, err := NewApple(stubAppleClient{}, "com.example.service", "https://api.example.com/v1/auth/apple/callback") + if err != nil { + t.Fatal(err) + } + + u, err := url.Parse(a.AuthCodeURL("state-value", "nonce-value", "verifier-value")) + if err != nil { + t.Fatal(err) + } + q := u.Query() + + if got := q.Get("response_mode"); got != "form_post" { + t.Errorf("response_mode = %q", got) + } + if got := q.Get("response_type"); got != "code" { + t.Errorf("response_type = %q", got) + } + if scope := q.Get("scope"); !strings.Contains(scope, "email") { + t.Errorf("scope = %q, want the email scope", scope) + } + if got := q.Get("nonce"); got != "nonce-value" { + t.Errorf("nonce = %q", got) + } + if got := q.Get("state"); got != "state-value" { + t.Errorf("state = %q", got) + } +} + +func TestCheckIdentityRejectsWhatTheProviderDoesNotVouchFor(t *testing.T) { + valid := func() *idtoken.Claims { + return &idtoken.Claims{Subject: "sub", Email: "person@example.com", EmailVerified: true, Nonce: "n"} + } + + if err := checkIdentity(valid(), "n"); err != nil { + t.Fatalf("a complete, verified identity was rejected: %v", err) + } + + replayed := valid() + replayed.Nonce = "someone-elses-nonce" + if err := checkIdentity(replayed, "n"); err == nil { + t.Error("a token minted for another authorization request was accepted") + } + if err := checkIdentity(valid(), ""); err == nil { + t.Error("an empty expected nonce was accepted, which would make the check a no-op") + } + + unverified := valid() + unverified.EmailVerified = false + if err := checkIdentity(unverified, "n"); err == nil { + t.Error("an unverified address was accepted") + } + + noSubject := valid() + noSubject.Subject = "" + if err := checkIdentity(noSubject, "n"); err == nil { + t.Error("a token with no subject was accepted") + } + + noEmail := valid() + noEmail.Email = "" + if err := checkIdentity(noEmail, "n"); err == nil { + t.Error("a token with no email claim was accepted") + } +} diff --git a/internal/app/socialauth/stub_test.go b/internal/app/socialauth/stub_test.go new file mode 100644 index 00000000..faa6b42c --- /dev/null +++ b/internal/app/socialauth/stub_test.go @@ -0,0 +1,13 @@ +package socialauth + +import apple "github.com/meszmate/apple-go" + +// stubAppleClient stands in for the Apple token endpoint. Only construction and +// URL building are exercised here; the exchange needs Apple's live keys. +type stubAppleClient struct{} + +func (stubAppleClient) ValidateCode(string) (*apple.TokenResponse, error) { return nil, nil } +func (stubAppleClient) ValidateCodeWithRedirectURI(string, string) (*apple.TokenResponse, error) { + return nil, nil +} +func (stubAppleClient) ValidateRefreshToken(string) (*apple.TokenResponse, error) { return nil, nil } diff --git a/internal/app/token/config.go b/internal/app/token/config.go index 9c05132f..efec9f5a 100644 --- a/internal/app/token/config.go +++ b/internal/app/token/config.go @@ -12,4 +12,7 @@ const ( AuthProviderApple = "apple" AuthProviderGoogle = "google" AuthProviderWebAuthn = "webauthn" + // AuthProviderOIDC covers generic single sign-on. Google and Apple keep + // their own values, so the security page can name what was actually used. + AuthProviderOIDC = "oidc" ) diff --git a/internal/config/auth.go b/internal/config/auth.go deleted file mode 100644 index 1fcb79e3..00000000 --- a/internal/config/auth.go +++ /dev/null @@ -1,20 +0,0 @@ -package config - -import ( - "os" - - "golang.org/x/oauth2" - "golang.org/x/oauth2/google" -) - -func GoogleOauth2Auth(baseURL string) *oauth2.Config { - return &oauth2.Config{ - RedirectURL: baseURL + "/auth/google/callback", - ClientID: os.Getenv("GOOGLE_CLIENT_ID"), - ClientSecret: os.Getenv("GOOGLE_CLIENT_SECRET"), - Scopes: []string{ - "https://www.googleapis.com/auth/userinfo.email", - }, - Endpoint: google.Endpoint, - } -} diff --git a/internal/config/config_auth.go b/internal/config/config_auth.go index 30956432..47ca2dfa 100644 --- a/internal/config/config_auth.go +++ b/internal/config/config_auth.go @@ -8,14 +8,20 @@ import ( ) type AuthConfig struct { + // Browser social sign-in. The redirect URIs are where the provider sends + // the browser back, which is the API, not the dashboard: both default to + // API_PUBLIC_URL plus /v1/auth//callback when left unset. GoogleClientID string GoogleRedirectURI string GoogleClientSecret string - AppleAppID string - AppleTeamID string - AppleKeyID string - AppleKeySecret string + // AppleAppID is the Services ID (the web identifier), not the app's + // bundle ID. + AppleAppID string + AppleTeamID string + AppleKeyID string + AppleKeySecret string + AppleRedirectURI string // Native-app (iOS) social sign-in. ID-token verification only needs the // expected audiences: the app's bundle ID for Sign in with Apple (fixed by @@ -59,6 +65,7 @@ func (c *Config) LoadAuthConfig(ctx context.Context) (*AuthConfig, error) { googleClientSecret := c.GetSecretOptional(ctx, "GOOGLE_CLIENT_SECRET", "google-auth/client_secret", "") appleAppID := c.GetStringOptional(ctx, "APPLE_APP_ID", "apple-auth/app_id", "") + appleRedirectURI := c.GetStringOptional(ctx, "APPLE_REDIRECT_URI", "apple-auth/redirect_uri", "") appleTeamID := c.GetStringOptional(ctx, "APPLE_TEAM_ID", "apple-auth/team_id", "") appleKeyID := c.GetStringOptional(ctx, "APPLE_KEY_ID", "apple-auth/key_id", "") appleKeySecret := c.GetSecretOptional(ctx, "APPLE_KEY_SECRET", "apple-auth/key_secret", "") @@ -88,10 +95,11 @@ func (c *Config) LoadAuthConfig(ctx context.Context) (*AuthConfig, error) { GoogleClientSecret: googleClientSecret, GoogleRedirectURI: googleRedirectURI, - AppleAppID: appleAppID, - AppleTeamID: appleTeamID, - AppleKeyID: appleKeyID, - AppleKeySecret: appleKeySecret, + AppleAppID: appleAppID, + AppleTeamID: appleTeamID, + AppleKeyID: appleKeyID, + AppleKeySecret: appleKeySecret, + AppleRedirectURI: appleRedirectURI, AppleIOSBundleID: appleIOSBundleID, GoogleIOSClientID: googleIOSClientID, diff --git a/internal/config/oauth2.go b/internal/config/oauth2.go index 4166fdad..86601afd 100644 --- a/internal/config/oauth2.go +++ b/internal/config/oauth2.go @@ -1,16 +1,15 @@ package config -import "golang.org/x/oauth2" - +// Oauth2 is the mailbox-connect OAuth configuration. Sign-in clients are not +// here: browser social sign-in builds its own config in internal/app/socialauth, +// where the ID token is verified rather than a userinfo endpoint called. type Oauth2 struct { - GoogleAuthorization *oauth2.Config - InboxAuthorization Oauth2Inbox + InboxAuthorization Oauth2Inbox } func LoadOauth2(baseURL string) *Oauth2 { return &Oauth2{ - GoogleAuthorization: GoogleOauth2Auth(baseURL), - InboxAuthorization: LoadOauth2Inbox(baseURL), + InboxAuthorization: LoadOauth2Inbox(baseURL), } } diff --git a/internal/models/auth.go b/internal/models/auth.go index d1757ad0..0934516c 100644 --- a/internal/models/auth.go +++ b/internal/models/auth.go @@ -1,18 +1,12 @@ package models -import ( - "github.com/meszmate/apple-go" - "github.com/meszmate/google-go" -) - -type ExternalAuth struct { - AppleAuth apple.AppleAuth - GoogleAuth *google.GoogleAuth -} - // ExternalAuthProviders is what GET /auth/providers advertises so one shipped // native app binary can discover which social sign-in options a (self-)hosted // backend supports. Client IDs here are public identifiers, not secrets. +// +// Browser social sign-in is separate and lives behind GET /auth/config: the +// dashboard needs to know which buttons to render, not which client to +// authenticate against, because the whole flow runs server-side. type ExternalAuthProviders struct { AppleBundleID string GoogleIOSClientID string diff --git a/internal/pkg/idtoken/idtoken.go b/internal/pkg/idtoken/idtoken.go index 362c0228..19389c0a 100644 --- a/internal/pkg/idtoken/idtoken.go +++ b/internal/pkg/idtoken/idtoken.go @@ -50,6 +50,11 @@ type Verifier struct { issuers []string audiences []string client *http.Client + // canonical collapses a provider that signs with more than one spelling of + // its issuer onto one value. Identities are keyed on (issuer, subject), so + // without it the same person becomes two accounts depending on which form + // the token happened to carry. + canonical string mu sync.Mutex keys map[string]*rsa.PublicKey @@ -67,12 +72,17 @@ func AppleVerifier(bundleIDs ...string) *Verifier { } // GoogleVerifier verifies Google Sign-In ID tokens for the given OAuth client IDs. +// +// Google signs with either spelling of its issuer, so both are accepted and the +// claims report the https form whichever arrived. func GoogleVerifier(clientIDs ...string) *Verifier { - return NewVerifier( + v := NewVerifier( "https://www.googleapis.com/oauth2/v3/certs", []string{"https://accounts.google.com", "accounts.google.com"}, clientIDs, ) + v.canonical = "https://accounts.google.com" + return v } func NewVerifier(jwksURL string, issuers, audiences []string) *Verifier { @@ -115,6 +125,10 @@ func (v *Verifier) Verify(ctx context.Context, raw string) (*Claims, error) { return nil, errors.New("idtoken: audience not allowed") } + if v.canonical != "" { + iss = v.canonical + } + sub, _ := mc["sub"].(string) email, _ := mc["email"].(string) given, _ := mc["given_name"].(string) diff --git a/web/src/app/app/settings/security/SessionManager.tsx b/web/src/app/app/settings/security/SessionManager.tsx index 268b3464..62cf658a 100644 --- a/web/src/app/app/settings/security/SessionManager.tsx +++ b/web/src/app/app/settings/security/SessionManager.tsx @@ -12,6 +12,7 @@ const PROVIDER_LABELS: Record = { email: "Email", google: "Google", apple: "Apple", + oidc: "Single sign-on", webauthn: "Passkey", }; diff --git a/web/src/app/auth/login/page.tsx b/web/src/app/auth/login/page.tsx index 65be4beb..af21b5b3 100644 --- a/web/src/app/auth/login/page.tsx +++ b/web/src/app/auth/login/page.tsx @@ -24,7 +24,7 @@ import getUser from "@/lib/api/client/auth/getUser"; import { WEBSITE_URL, TURNSTILE_KEY, API_URL } from "@/lib/information"; import useAuthConfig from "@/lib/api/hooks/auth/useAuthConfig"; import type Session from "@/lib/api/models/auth/Session"; -import beginOIDC from "@/lib/api/client/auth/beginOIDC"; +import beginSSO from "@/lib/api/client/auth/beginSSO"; import type { AppError } from "@/lib/api/client/normalizeError"; import buildError from "@/lib/helper/buildError"; import * as Sentry from "@sentry/react"; @@ -188,8 +188,19 @@ export default function LoginPage() { ? (import.meta.env.VITE_TURNSTILE_BYPASS_TOKEN?.trim() || "") : ""; + // A social or single sign-on login that hits an enrolled TOTP comes back + // here with its pending challenge rather than a session, because the 2FA + // form lives on this screen. See the SSO landing page. + const ssoTwoFA = (location.state as { two_fa_pending?: string } | null)?.two_fa_pending ?? ""; + // The pending token is single use, so it must not survive a reload of this + // screen: history state does, and would leave a form that can only fail. + useEffect(() => { + if (ssoTwoFA) window.history.replaceState(null, "", location.pathname + location.search); + // eslint-disable-next-line react-hooks/exhaustive-deps + }, []); + /* State */ - const [step, setStep] = useState("email"); + const [step, setStep] = useState(ssoTwoFA ? "2fa" : "email"); const [mode, setMode] = useState<"signin" | "signup">(() => location.pathname.includes("/register") || new URLSearchParams(location.search).get("mode") === "signup" @@ -231,7 +242,7 @@ export default function LoginPage() { ); const [password, setPassword] = useState(""); const [session, setSession] = useState(""); - const [pendingToken, setPendingToken] = useState(""); + const [pendingToken, setPendingToken] = useState(ssoTwoFA); const [direction, setDirection] = useState(0); const pendingRef = useRef<((token: string) => void) | null>(null); const tokenRef = useRef(""); @@ -487,16 +498,19 @@ export default function LoginPage() { return true; }, [completeSession]); - /* Single sign-on. The backend mints state, nonce and the PKCE verifier and - stores them server-side, so the client only needs the URL. */ - const handleSSO = useCallback(async () => { + /* Browser sign-in with a provider: "oidc", "google" or "apple". The backend + mints state, nonce and the PKCE verifier and stores them server-side, so + the client only needs the URL. The whole page navigates rather than + opening a popup, and the provider returns to /auth/sso. */ + const handleProvider = useCallback(async (provider: string) => { try { - const { url } = await beginOIDC(); + const { url } = await beginSSO(provider); window.location.href = url; } catch (e) { toast.error(buildError(e as AppError)); } }, []); + const handleSSO = useCallback(() => handleProvider("oidc"), [handleProvider]); /* ── Step 1: Email ─────────────────────── */ const handleEmailContinue = (data: z.infer) => { @@ -671,6 +685,7 @@ export default function LoginPage() { invited={!!inviteToken} providers={authConfig.providers} passkeysEnabled={passkeysEnabled} + onProvider={handleProvider} onModeChange={handleModeChange} defaultEmail={email} onContinue={handleEmailContinue} @@ -688,6 +703,7 @@ export default function LoginPage() { email={email} pending={pending} ssoEnabled={authConfig.providers.includes("oidc")} + ssoLabel={authConfig.provider_labels?.oidc} onSSO={handleSSO} onBack={() => goTo("email", -1)} onSubmit={handleSignIn} @@ -808,6 +824,7 @@ function EmailStep({ invited, providers, passkeysEnabled, + onProvider, onModeChange, defaultEmail, onContinue, @@ -822,6 +839,7 @@ function EmailStep({ invited: boolean; providers: string[]; passkeysEnabled: boolean; + onProvider: (provider: string) => Promise | void; onModeChange: (m: "signin" | "signup") => void; defaultEmail: string; onContinue: (data: z.infer) => void; @@ -934,6 +952,7 @@ function EmailStep({ void; onBack: () => void; onSubmit: (data: z.infer) => void; @@ -1107,7 +1128,7 @@ function SignInStep({ disabled={pending} className="w-full h-10 rounded-md border border-slate-200 text-[13px] font-medium text-slate-700 hover:bg-slate-50 focus:border-sky-400 focus:ring-2 focus:ring-sky-100 transition-colors disabled:opacity-50" > - Continue with single sign-on + Continue with {ssoLabel || "single sign-on"} )} diff --git a/web/src/app/auth/sso/page.tsx b/web/src/app/auth/sso/page.tsx index d69a8f1c..9d46d9f6 100644 --- a/web/src/app/auth/sso/page.tsx +++ b/web/src/app/auth/sso/page.tsx @@ -38,6 +38,24 @@ export default function SSOCallbackPage() { (async () => { try { const session = await exchangeSSO(code); + + // A provider-verified identity does not clear an enrolled + // second factor, so this can come back as a challenge instead + // of a session. The 2FA form lives on the login screen, which + // picks the challenge up from history state. + if (session.two_fa_required) { + if (!session.pending_token) { + setError("Two-factor authentication is required, but the challenge did not arrive. Try signing in again."); + return; + } + navigate("/auth/login", { replace: true, state: { two_fa_pending: session.pending_token } }); + return; + } + if (!session.access_token) { + setError("That sign-in did not return a session. Try again."); + return; + } + saveTokens(session as unknown as Record); queryClient.clear(); try { diff --git a/web/src/components/auth/external.tsx b/web/src/components/auth/external.tsx index 2dd0f9b9..547a88c4 100644 --- a/web/src/components/auth/external.tsx +++ b/web/src/components/auth/external.tsx @@ -1,9 +1,8 @@ -import React, { useEffect, useRef } from "react"; +import React, { useEffect, useRef, useState } from "react"; import { motion, AnimatePresence } from "motion/react"; import { RiAppleFill } from "@remixicon/react"; import { KeyRound, Loader2Icon } from "lucide-react"; import { Google } from "../svg"; -import { API_URL, PopupCenter } from "@/lib/information"; import { AUTH_CELL as CELL } from "./styles"; // Snappy-but-smooth spring used for the layout reflow when the passkey cell @@ -35,19 +34,39 @@ function CellBody({ children }: { children: React.ReactNode }) { // equal-width cell; on create-account it animates out and Google/Apple glide // wider to fill — one fluid layout transition, no snap. // -// `providers` is the list GET /auth/config returns. A button for a provider -// this deployment has no client for is a dead end, so it is never rendered. +// `providers` is the list GET /auth/config returns, and it holds only the +// providers this backend can actually complete a sign-in with. A button for a +// provider it cannot is a dead end, so it is never rendered. +// +// `onProvider` navigates the whole page to the provider rather than opening a +// popup: a popup is blocked by default on iOS Safari and lands the session in a +// window the app cannot read. export default function ExternalLogin({ passkey, providers, + onProvider, }: { passkey?: { onClick: () => void; onPrepare: () => void; loading: boolean; disabled?: boolean; label?: string }; providers: string[]; + onProvider: (provider: string) => Promise | void; }) { const passkeyRef = useRef(null); + // Which provider is mid-handoff. The click ends in a full page navigation, + // so without this the button sits inert for the length of a round trip. + const [busy, setBusy] = useState(null); const google = providers.includes("google"); const apple = providers.includes("apple"); + const start = async (provider: string) => { + if (busy) return; + setBusy(provider); + try { + await onProvider(provider); + } finally { + setBusy(null); + } + }; + useEffect(() => { const button = passkeyRef.current; if (!button || !passkey) return; @@ -121,11 +140,17 @@ export default function ExternalLogin({ key="google" type="button" transition={spring} - onClick={() => PopupCenter(`${API_URL}/auth/google/login`, "Google Login")} - className={`${CELL} flex-1 min-w-0`} + disabled={busy !== null} + aria-busy={busy === "google"} + onClick={() => start("google")} + className={`${CELL} flex-1 min-w-0 ${busy === "google" ? "!opacity-100" : ""}`} > - + {busy === "google" ? ( + + ) : ( + + )} Google @@ -137,11 +162,17 @@ export default function ExternalLogin({ key="apple" type="button" transition={spring} - onClick={() => PopupCenter(`${API_URL}/auth/apple/login`, "Apple Login")} - className={`${CELL} flex-1 min-w-0`} + disabled={busy !== null} + aria-busy={busy === "apple"} + onClick={() => start("apple")} + className={`${CELL} flex-1 min-w-0 ${busy === "apple" ? "!opacity-100" : ""}`} > - + {busy === "apple" ? ( + + ) : ( + + )} Apple diff --git a/web/src/lib/api/client/auth/beginOIDC.ts b/web/src/lib/api/client/auth/beginOIDC.ts deleted file mode 100644 index 9931b7e3..00000000 --- a/web/src/lib/api/client/auth/beginOIDC.ts +++ /dev/null @@ -1,13 +0,0 @@ -import Request from "../Request"; - -/** - * Starts a generic OpenID Connect sign-in. The backend returns the - * authorization URL rather than redirecting, because the dashboard is a - * single-page app on a different origin from the API. - */ -export default async function beginOIDC(): Promise<{ url: string }> { - return await Request<{ url: string }>({ - method: "POST", - url: "/auth/oidc/begin", - }); -} diff --git a/web/src/lib/api/client/auth/beginSSO.ts b/web/src/lib/api/client/auth/beginSSO.ts new file mode 100644 index 00000000..8041ba3c --- /dev/null +++ b/web/src/lib/api/client/auth/beginSSO.ts @@ -0,0 +1,16 @@ +import Request from "../Request"; + +/** + * Starts a browser sign-in with one provider: "oidc", "google" or "apple". + * + * The backend returns the authorization URL rather than redirecting, because + * the dashboard is a single-page app on a different origin from the API. The + * state, nonce and PKCE verifier are minted and held server-side, so nothing + * the client holds can be replayed. + */ +export default async function beginSSO(provider: string): Promise<{ url: string }> { + return await Request<{ url: string }>({ + method: "POST", + url: `/auth/${provider}/begin`, + }); +} diff --git a/web/src/lib/api/client/auth/exchangeSSO.ts b/web/src/lib/api/client/auth/exchangeSSO.ts index 8c167901..4e5c484b 100644 --- a/web/src/lib/api/client/auth/exchangeSSO.ts +++ b/web/src/lib/api/client/auth/exchangeSSO.ts @@ -5,12 +5,13 @@ import Request from "../Request"; * Swaps the single-use code from an SSO redirect for the real session. * * The backend holds the session and hands back only an opaque code, so no token - * ever lands in a URL, browser history or proxy log. + * ever lands in a URL, browser history or proxy log. The result is a login like + * any other, so it can also come back as a 2FA challenge. */ -export default async function exchangeSSO(code: string): Promise { - return await Request({ +export default async function exchangeSSO(code: string): Promise { + return await Request({ method: "POST", - url: "/auth/oidc/exchange", + url: "/auth/sso/exchange", data: { code }, }); } diff --git a/web/src/lib/api/models/auth/AuthConfig.ts b/web/src/lib/api/models/auth/AuthConfig.ts index 2788390b..859e2764 100644 --- a/web/src/lib/api/models/auth/AuthConfig.ts +++ b/web/src/lib/api/models/auth/AuthConfig.ts @@ -15,6 +15,10 @@ export default interface AuthConfig { mail_delivers: boolean; passkeys: boolean; providers: string[]; + /** What each provider's button should say, keyed by the same identifiers. + * Absent for a backend that predates it, so callers fall back to a name + * of their own. */ + provider_labels?: Record; self_hosted: boolean; /** False when BILLING_PROVIDER=none: the backend unlocks every feature and * the org must not be presented as being on a trial or free tier. */ diff --git a/web/src/lib/information.ts b/web/src/lib/information.ts index c2df64fd..bb0ddb72 100644 --- a/web/src/lib/information.ts +++ b/web/src/lib/information.ts @@ -24,34 +24,3 @@ export const CREATING = "Creating..."; export const CREATED = "Successfully created." export const ADDING = "Adding..." export const ADDED = "Successfully added." - -// MAILBOX -export const OUTLOOK_BOX_AUTH = API_URL + "/emails/outlook/login"; -export const GOOGLE_BOX_AUTH = API_URL + "/emails/google/login"; - -export function PopupCenter(url: string, title: string) { - const dualScreenLeft = window.screenLeft ?? window.screenX; - const dualScreenTop = window.screenTop ?? window.screenY; - - const width = - window.innerWidth ?? document.documentElement.clientWidth ?? screen.width; - - const height = - window.innerHeight ?? - document.documentElement.clientHeight ?? - screen.height; - - const systemZoom = width / window.screen.availWidth; - - const left = (width - 500) / 2 / systemZoom + dualScreenLeft; - const top = (height - 550) / 2 / systemZoom + dualScreenTop; - - const newWindow = window.open( - url, - title, - `width=${500 / systemZoom},height=${550 / systemZoom - },top=${top},left=${left}` - ); - - newWindow?.focus(); -};