fix(app): cap a web session at its token's expiry and sign out on owner-token revoke
A login link made by an expiring token opened a 12 hour session that outlived it. The link now carries min(now+SessionTTL, token expiry) and the session and cookie use it as their exp; root and tokens that never expire are not capped. Revoking a token with owner rights bumps the session epoch, which signs every browser out, since sessions are not tied to the token that opened them. 'token revoke' says so in its help.
- commit
- 6833c9f5e1d4c1a945393a15ec2faf1c725bd849
- author
- brice · · 2026-10-10 19:26 UTC
- committer
- brice · · 2026-10-10 19:28 UTC
- parent
- 12ce961
- signature
- GPG Verified
CD88F9E564E0102646FB6077A1A07C7C7C3E349D· arch-gpg Verified for the committer email
8 files · +260 -40
M cmd/ermite/cli_token.go +5 -2
| @@ -222,8 +222,11 @@ func newTokenRevokeCmd(env *cliEnv) *cobra.Command { | |||
| 222 | 222 | return &cobra.Command{ | |
| 223 | 223 | Use: "revoke <id>", | |
| 224 | 224 | Short: "Revoke a token", | |
| 225 | - | Long: "Revoke a token by id (see `ermite token list`). It stops working immediately.", | |
| 226 | - | Args: cobra.ExactArgs(1), | |
| 225 | + | Long: "Revoke a token by id (see `ermite token list`). It stops working immediately.\n\n" + | |
| 226 | + | "A token with every scope and no --repo list has the rights of the owner (it can open web\n" + | |
| 227 | + | "sessions). Revoking one also signs every browser out, like `ermite web logout --all`:\n" + | |
| 228 | + | "web sessions are not tied to the token that opened them. Sign in again with `ermite web login`.", | |
| 229 | + | Args: cobra.ExactArgs(1), | |
| 227 | 230 | ValidArgsFunction: func(cmd *cobra.Command, args []string, toComplete string) ([]string, cobra.ShellCompDirective) { | |
| 228 | 231 | if len(args) > 0 { | |
| 229 | 232 | return nil, cobra.ShellCompDirectiveNoFileComp | |
| @@ -222,8 +222,11 @@ func newTokenRevokeCmd(env *cliEnv) *cobra.Command { | |||||
| 222 | return &cobra.Command{ | 222 | return &cobra.Command{ | ||
| 223 | Use: "revoke <id>", | 223 | Use: "revoke <id>", | ||
| 224 | Short: "Revoke a token", | 224 | Short: "Revoke a token", | ||
| 225 | - | Long: "Revoke a token by id (see `ermite token list`). It stops working immediately.", | 225 | + | Long: "Revoke a token by id (see `ermite token list`). It stops working immediately.\n\n" + |
| 226 | - | Args: cobra.ExactArgs(1), | 226 | + | "A token with every scope and no --repo list has the rights of the owner (it can open web\n" + |
| 227 | + | "sessions). Revoking one also signs every browser out, like `ermite web logout --all`:\n" + | |||
| 228 | + | "web sessions are not tied to the token that opened them. Sign in again with `ermite web login`.", | |||
| 229 | + | Args: cobra.ExactArgs(1), | |||
| 227 | ValidArgsFunction: func(cmd *cobra.Command, args []string, toComplete string) ([]string, cobra.ShellCompDirective) { | 230 | ValidArgsFunction: func(cmd *cobra.Command, args []string, toComplete string) ([]string, cobra.ShellCompDirective) { | ||
| 228 | if len(args) > 0 { | 231 | if len(args) > 0 { | ||
| 229 | return nil, cobra.ShellCompDirectiveNoFileComp | 232 | return nil, cobra.ShellCompDirectiveNoFileComp | ||
M cmd/ermite/forge.go +1 -1
| @@ -147,7 +147,7 @@ func newForge(ctx context.Context, cfg config.Config, opts options) (_ *forge, e | |||
| 147 | 147 | Int("invalid_dirs", len(report.InvalidDirs)). | |
| 148 | 148 | Msg("repositories reconciled") | |
| 149 | 149 | ||
| 150 | - | f.Tokens = app.NewTokens(store.Tokens(), clock) | |
| 150 | + | f.Tokens = app.NewTokens(store.Tokens(), clock, store.Sessions()) | |
| 151 | 151 | f.Auth = app.NewAuth(cfg.AdminTokenHash, store.Tokens(), clock) | |
| 152 | 152 | f.maint = app.NewMaintainer(ctx, git, opts.maintenanceDelay) | |
| 153 | 153 | f.Pushes = app.NewPushes(store, f.Repos, git, f.Bus, clock, f.maint, f.Auth) | |
| @@ -147,7 +147,7 @@ func newForge(ctx context.Context, cfg config.Config, opts options) (_ *forge, e | |||||
| 147 | Int("invalid_dirs", len(report.InvalidDirs)). | 147 | Int("invalid_dirs", len(report.InvalidDirs)). | ||
| 148 | Msg("repositories reconciled") | 148 | Msg("repositories reconciled") | ||
| 149 | 149 | ||||
| 150 | - | f.Tokens = app.NewTokens(store.Tokens(), clock) | 150 | + | f.Tokens = app.NewTokens(store.Tokens(), clock, store.Sessions()) |
| 151 | f.Auth = app.NewAuth(cfg.AdminTokenHash, store.Tokens(), clock) | 151 | f.Auth = app.NewAuth(cfg.AdminTokenHash, store.Tokens(), clock) | ||
| 152 | f.maint = app.NewMaintainer(ctx, git, opts.maintenanceDelay) | 152 | f.maint = app.NewMaintainer(ctx, git, opts.maintenanceDelay) | ||
| 153 | f.Pushes = app.NewPushes(store, f.Repos, git, f.Bus, clock, f.maint, f.Auth) | 153 | f.Pushes = app.NewPushes(store, f.Repos, git, f.Bus, clock, f.maint, f.Auth) | ||
M cmd/ermite/web.go +16 -2
| @@ -51,11 +51,25 @@ func newWeb(cfg config.Config, api *rpc.API, sessions *app.Sessions, limiter *cl | |||
| 51 | 51 | type webSessions struct{ *app.Sessions } | |
| 52 | 52 | ||
| 53 | 53 | func (s webSessions) Login(ctx context.Context, nonce string) (string, time.Duration, error) { | |
| 54 | - | token, _, err := s.Sessions.Login(ctx, nonce) | |
| 54 | + | token, expiresAt, err := s.Sessions.Login(ctx, nonce) | |
| 55 | 55 | if errors.Is(err, domain.ErrUnauthenticated) { | |
| 56 | 56 | return "", 0, web.ErrLinkRejected | |
| 57 | 57 | } | |
| 58 | - | return token, app.SessionTTL, err | |
| 58 | + | if err != nil { | |
| 59 | + | return "", 0, err | |
| 60 | + | } | |
| 61 | + | // The cookie lasts as long as the session, which the expiry of the token | |
| 62 | + | // that made the link may cut short. The session token carries its own | |
| 63 | + | // expiry, so a cookie that outlives it by under a second is harmless. | |
| 64 | + | // The expiry is truncated to the second, hence the slack on the full TTL. | |
| 65 | + | lifetime := app.SessionTTL | |
| 66 | + | if d := time.Until(expiresAt); d < app.SessionTTL-time.Second { | |
| 67 | + | lifetime = d.Round(time.Second) | |
| 68 | + | } | |
| 69 | + | if lifetime < time.Second { | |
| 70 | + | return "", 0, web.ErrLinkRejected | |
| 71 | + | } | |
| 72 | + | return token, lifetime, nil | |
| 59 | 73 | } | |
| 60 | 74 | ||
| 61 | 75 | func (s webSessions) Verify(ctx context.Context, token string) (context.Context, bool, error) { | |
| @@ -51,11 +51,25 @@ func newWeb(cfg config.Config, api *rpc.API, sessions *app.Sessions, limiter *cl | |||||
| 51 | type webSessions struct{ *app.Sessions } | 51 | type webSessions struct{ *app.Sessions } | ||
| 52 | 52 | ||||
| 53 | func (s webSessions) Login(ctx context.Context, nonce string) (string, time.Duration, error) { | 53 | func (s webSessions) Login(ctx context.Context, nonce string) (string, time.Duration, error) { | ||
| 54 | - | token, _, err := s.Sessions.Login(ctx, nonce) | 54 | + | token, expiresAt, err := s.Sessions.Login(ctx, nonce) |
| 55 | if errors.Is(err, domain.ErrUnauthenticated) { | 55 | if errors.Is(err, domain.ErrUnauthenticated) { | ||
| 56 | return "", 0, web.ErrLinkRejected | 56 | return "", 0, web.ErrLinkRejected | ||
| 57 | } | 57 | } | ||
| 58 | - | return token, app.SessionTTL, err | 58 | + | if err != nil { |
| 59 | + | return "", 0, err | |||
| 60 | + | } | |||
| 61 | + | // The cookie lasts as long as the session, which the expiry of the token | |||
| 62 | + | // that made the link may cut short. The session token carries its own | |||
| 63 | + | // expiry, so a cookie that outlives it by under a second is harmless. | |||
| 64 | + | // The expiry is truncated to the second, hence the slack on the full TTL. | |||
| 65 | + | lifetime := app.SessionTTL | |||
| 66 | + | if d := time.Until(expiresAt); d < app.SessionTTL-time.Second { | |||
| 67 | + | lifetime = d.Round(time.Second) | |||
| 68 | + | } | |||
| 69 | + | if lifetime < time.Second { | |||
| 70 | + | return "", 0, web.ErrLinkRejected | |||
| 71 | + | } | |||
| 72 | + | return token, lifetime, nil | |||
| 59 | } | 73 | } | ||
| 60 | 74 | ||||
| 61 | func (s webSessions) Verify(ctx context.Context, token string) (context.Context, bool, error) { | 75 | func (s webSessions) Verify(ctx context.Context, token string) (context.Context, bool, error) { | ||
M cmd/ermite/web_session_test.go +61 -0
| @@ -17,6 +17,7 @@ import ( | |||
| 17 | 17 | "github.com/stretchr/testify/require" | |
| 18 | 18 | ||
| 19 | 19 | pb "bnema.dev/ermite-api/gen/ermite" | |
| 20 | + | "bnema.dev/ermite/internal/app" | |
| 20 | 21 | "bnema.dev/ermite/internal/domain" | |
| 21 | 22 | ) | |
| 22 | 23 | ||
| @@ -282,6 +283,66 @@ func validNonceShape(p string) bool { | |||
| 282 | 283 | return len(strings.TrimPrefix(p, "/-/login/")) == 43 | |
| 283 | 284 | } | |
| 284 | 285 | ||
| 286 | + | // tokenID finds the id of the stored token with that secret, by listing them | |
| 287 | + | // as root and matching the hash. | |
| 288 | + | func (c *cliHarness) tokenID(t *testing.T, secret string) string { | |
| 289 | + | t.Helper() | |
| 290 | + | all, err := c.forge.Tokens.List(c.ctx, domain.Root()) | |
| 291 | + | require.NoError(t, err) | |
| 292 | + | for _, tok := range all { | |
| 293 | + | if tok.Hash == domain.HashTokenSecret(secret) { | |
| 294 | + | return tok.ID | |
| 295 | + | } | |
| 296 | + | } | |
| 297 | + | t.Fatal("token not found") | |
| 298 | + | return "" | |
| 299 | + | } | |
| 300 | + | ||
| 301 | + | // GO-003: a session opened with a link made by an expiring token ends with the | |
| 302 | + | // token, and revoking a token with owner rights signs every browser out; any | |
| 303 | + | // other token leaves the sessions alone. | |
| 304 | + | func TestWebRevokingAnOwnerTokenSignsEveryoneOut(t *testing.T) { | |
| 305 | + | c := newCLIHarness(t) | |
| 306 | + | c.seed("secret", domain.VisibilityPrivate, "notes.txt") | |
| 307 | + | b := browser{c.harness} | |
| 308 | + | ||
| 309 | + | // The cap: a link made by a token that expires in an hour gives a cookie, | |
| 310 | + | // and a session, of an hour. | |
| 311 | + | soon, err := c.forge.Tokens.Create(c.ctx, domain.Root(), app.CreateTokenParams{ | |
| 312 | + | Label: "soon", Scopes: domain.AllScopes(), ExpiresAt: time.Now().Add(time.Hour), | |
| 313 | + | }) | |
| 314 | + | require.NoError(t, err) | |
| 315 | + | res := c.cli(t, soon.Secret, "web", "login").ok(t) | |
| 316 | + | login := b.do("POST", loginPath(t, res.stdout), map[string]string{"Origin": publicOrigin}, "") | |
| 317 | + | require.Equal(t, http.StatusSeeOther, login.status, login.body) | |
| 318 | + | ck := login.cookie(sessionCookie) | |
| 319 | + | require.NotNil(t, ck) | |
| 320 | + | assert.InDelta(t, 3600, ck.MaxAge, 5, "the cookie lasts as long as the token") | |
| 321 | + | claims := jwt.MapClaims{} | |
| 322 | + | _, _, err = jwt.NewParser().ParseUnverified(ck.Value, claims) | |
| 323 | + | require.NoError(t, err) | |
| 324 | + | exp, err := claims.GetExpirationTime() | |
| 325 | + | require.NoError(t, err) | |
| 326 | + | assert.WithinDuration(t, soon.Token.ExpiresAt, exp.Time, 2*time.Second) | |
| 327 | + | ||
| 328 | + | // Revoking a token that cannot own the forge keeps the sessions. | |
| 329 | + | session := c.signIn(t) | |
| 330 | + | partial := c.token(domain.AllScopes()[:8]) | |
| 331 | + | c.cli(t, adminToken, "token", "revoke", c.tokenID(t, partial)).ok(t) | |
| 332 | + | assert.Equal(t, 200, b.get("/secret", as(session)).status) | |
| 333 | + | assert.Equal(t, 200, b.get("/secret", as(ck.Value)).status) | |
| 334 | + | ||
| 335 | + | // Revoking one that can signs everyone out, whatever token made the link. | |
| 336 | + | res = c.cli(t, adminToken, "token", "revoke", c.tokenID(t, soon.Secret)).ok(t) | |
| 337 | + | assert.Contains(t, res.stdout, "revoked") | |
| 338 | + | assert.Equal(t, 404, b.get("/secret", as(session)).status) | |
| 339 | + | assert.Equal(t, 404, b.get("/secret", as(ck.Value)).status) | |
| 340 | + | ||
| 341 | + | // The help says so. | |
| 342 | + | help := c.cli(t, adminToken, "token", "revoke", "--help").ok(t) | |
| 343 | + | assert.Contains(t, help.stdout+help.stderr, "signs every browser out") | |
| 344 | + | } | |
| 345 | + | ||
| 285 | 346 | // cookieInterceptor makes a Connect client send the session cookie, as a | |
| 286 | 347 | // browser-made call would, to show the API pays no attention to it. | |
| 287 | 348 | func cookieInterceptor(token string) connect.Interceptor { | |
| @@ -17,6 +17,7 @@ import ( | |||||
| 17 | "github.com/stretchr/testify/require" | 17 | "github.com/stretchr/testify/require" | ||
| 18 | 18 | ||||
| 19 | pb "bnema.dev/ermite-api/gen/ermite" | 19 | pb "bnema.dev/ermite-api/gen/ermite" | ||
| 20 | + | "bnema.dev/ermite/internal/app" | |||
| 20 | "bnema.dev/ermite/internal/domain" | 21 | "bnema.dev/ermite/internal/domain" | ||
| 21 | ) | 22 | ) | ||
| 22 | 23 | ||||
| @@ -282,6 +283,66 @@ func validNonceShape(p string) bool { | |||||
| 282 | return len(strings.TrimPrefix(p, "/-/login/")) == 43 | 283 | return len(strings.TrimPrefix(p, "/-/login/")) == 43 | ||
| 283 | } | 284 | } | ||
| 284 | 285 | ||||
| 286 | + | // tokenID finds the id of the stored token with that secret, by listing them | |||
| 287 | + | // as root and matching the hash. | |||
| 288 | + | func (c *cliHarness) tokenID(t *testing.T, secret string) string { | |||
| 289 | + | t.Helper() | |||
| 290 | + | all, err := c.forge.Tokens.List(c.ctx, domain.Root()) | |||
| 291 | + | require.NoError(t, err) | |||
| 292 | + | for _, tok := range all { | |||
| 293 | + | if tok.Hash == domain.HashTokenSecret(secret) { | |||
| 294 | + | return tok.ID | |||
| 295 | + | } | |||
| 296 | + | } | |||
| 297 | + | t.Fatal("token not found") | |||
| 298 | + | return "" | |||
| 299 | + | } | |||
| 300 | + | ||||
| 301 | + | // GO-003: a session opened with a link made by an expiring token ends with the | |||
| 302 | + | // token, and revoking a token with owner rights signs every browser out; any | |||
| 303 | + | // other token leaves the sessions alone. | |||
| 304 | + | func TestWebRevokingAnOwnerTokenSignsEveryoneOut(t *testing.T) { | |||
| 305 | + | c := newCLIHarness(t) | |||
| 306 | + | c.seed("secret", domain.VisibilityPrivate, "notes.txt") | |||
| 307 | + | b := browser{c.harness} | |||
| 308 | + | ||||
| 309 | + | // The cap: a link made by a token that expires in an hour gives a cookie, | |||
| 310 | + | // and a session, of an hour. | |||
| 311 | + | soon, err := c.forge.Tokens.Create(c.ctx, domain.Root(), app.CreateTokenParams{ | |||
| 312 | + | Label: "soon", Scopes: domain.AllScopes(), ExpiresAt: time.Now().Add(time.Hour), | |||
| 313 | + | }) | |||
| 314 | + | require.NoError(t, err) | |||
| 315 | + | res := c.cli(t, soon.Secret, "web", "login").ok(t) | |||
| 316 | + | login := b.do("POST", loginPath(t, res.stdout), map[string]string{"Origin": publicOrigin}, "") | |||
| 317 | + | require.Equal(t, http.StatusSeeOther, login.status, login.body) | |||
| 318 | + | ck := login.cookie(sessionCookie) | |||
| 319 | + | require.NotNil(t, ck) | |||
| 320 | + | assert.InDelta(t, 3600, ck.MaxAge, 5, "the cookie lasts as long as the token") | |||
| 321 | + | claims := jwt.MapClaims{} | |||
| 322 | + | _, _, err = jwt.NewParser().ParseUnverified(ck.Value, claims) | |||
| 323 | + | require.NoError(t, err) | |||
| 324 | + | exp, err := claims.GetExpirationTime() | |||
| 325 | + | require.NoError(t, err) | |||
| 326 | + | assert.WithinDuration(t, soon.Token.ExpiresAt, exp.Time, 2*time.Second) | |||
| 327 | + | ||||
| 328 | + | // Revoking a token that cannot own the forge keeps the sessions. | |||
| 329 | + | session := c.signIn(t) | |||
| 330 | + | partial := c.token(domain.AllScopes()[:8]) | |||
| 331 | + | c.cli(t, adminToken, "token", "revoke", c.tokenID(t, partial)).ok(t) | |||
| 332 | + | assert.Equal(t, 200, b.get("/secret", as(session)).status) | |||
| 333 | + | assert.Equal(t, 200, b.get("/secret", as(ck.Value)).status) | |||
| 334 | + | ||||
| 335 | + | // Revoking one that can signs everyone out, whatever token made the link. | |||
| 336 | + | res = c.cli(t, adminToken, "token", "revoke", c.tokenID(t, soon.Secret)).ok(t) | |||
| 337 | + | assert.Contains(t, res.stdout, "revoked") | |||
| 338 | + | assert.Equal(t, 404, b.get("/secret", as(session)).status) | |||
| 339 | + | assert.Equal(t, 404, b.get("/secret", as(ck.Value)).status) | |||
| 340 | + | ||||
| 341 | + | // The help says so. | |||
| 342 | + | help := c.cli(t, adminToken, "token", "revoke", "--help").ok(t) | |||
| 343 | + | assert.Contains(t, help.stdout+help.stderr, "signs every browser out") | |||
| 344 | + | } | |||
| 345 | + | ||||
| 285 | // cookieInterceptor makes a Connect client send the session cookie, as a | 346 | // cookieInterceptor makes a Connect client send the session cookie, as a | ||
| 286 | // browser-made call would, to show the API pays no attention to it. | 347 | // browser-made call would, to show the API pays no attention to it. | ||
| 287 | func cookieInterceptor(token string) connect.Interceptor { | 348 | func cookieInterceptor(token string) connect.Interceptor { | ||
M internal/app/sessions.go +32 -13
| @@ -56,7 +56,15 @@ type Sessions struct { | |||
| 56 | 56 | key []byte | |
| 57 | 57 | ||
| 58 | 58 | mu sync.Mutex | |
| 59 | - | links map[[sha256.Size]byte]time.Time // by hash of the nonce, to the expiry | |
| 59 | + | links map[[sha256.Size]byte]pendingLink // by hash of the nonce | |
| 60 | + | } | |
| 61 | + | ||
| 62 | + | // pendingLink is a login link that waits to be used. | |
| 63 | + | type pendingLink struct { | |
| 64 | + | expires time.Time | |
| 65 | + | // sessionCap is the latest moment the session it opens may last until: the | |
| 66 | + | // expiry of the token that created the link. Zero means no cap. | |
| 67 | + | sessionCap time.Time | |
| 60 | 68 | } | |
| 61 | 69 | ||
| 62 | 70 | // NewSessions builds the session use cases and loads the signing key, which | |
| @@ -67,7 +75,7 @@ func NewSessions(ctx context.Context, store ports.SessionStore, clock ports.Cloc | |||
| 67 | 75 | if err != nil { | |
| 68 | 76 | return nil, fmt.Errorf("load session key: %w", err) | |
| 69 | 77 | } | |
| 70 | - | return &Sessions{store: store, clock: clock, publicURL: publicURL, key: key, links: map[[sha256.Size]byte]time.Time{}}, nil | |
| 78 | + | return &Sessions{store: store, clock: clock, publicURL: publicURL, key: key, links: map[[sha256.Size]byte]pendingLink{}}, nil | |
| 71 | 79 | } | |
| 72 | 80 | ||
| 73 | 81 | // requireOwner is the access rule of the session API: root, or a stored token | |
| @@ -83,7 +91,9 @@ func requireOwner(p domain.Principal) error { | |||
| 83 | 91 | } | |
| 84 | 92 | ||
| 85 | 93 | // CreateLoginLink makes a link valid for LoginLinkTTL and usable once. Only | |
| 86 | - | // the owner may. It gives domain.ErrResourceExhausted when MaxPendingLoginLinks | |
| 94 | + | // the owner may. The session the link opens never outlives the token that | |
| 95 | + | // created it: it is capped at that token's expiry (no cap for root, or a token | |
| 96 | + | // that never expires). It gives domain.ErrResourceExhausted when MaxPendingLoginLinks | |
| 87 | 97 | // links wait. The nonce is never logged. | |
| 88 | 98 | func (s *Sessions) CreateLoginLink(ctx context.Context, p domain.Principal) (LoginLink, error) { | |
| 89 | 99 | if err := requireOwner(p); err != nil { | |
| @@ -98,8 +108,8 @@ func (s *Sessions) CreateLoginLink(ctx context.Context, p domain.Principal) (Log | |||
| 98 | 108 | exp := now.Add(LoginLinkTTL) | |
| 99 | 109 | ||
| 100 | 110 | s.mu.Lock() | |
| 101 | - | for h, e := range s.links { | |
| 102 | - | if !now.Before(e) { | |
| 111 | + | for h, l := range s.links { | |
| 112 | + | if !now.Before(l.expires) { | |
| 103 | 113 | delete(s.links, h) | |
| 104 | 114 | } | |
| 105 | 115 | } | |
| @@ -107,7 +117,7 @@ func (s *Sessions) CreateLoginLink(ctx context.Context, p domain.Principal) (Log | |||
| 107 | 117 | s.mu.Unlock() | |
| 108 | 118 | return LoginLink{}, fmt.Errorf("%w: %d login links are already pending, use one or wait a minute", domain.ErrResourceExhausted, MaxPendingLoginLinks) | |
| 109 | 119 | } | |
| 110 | - | s.links[sha256.Sum256([]byte(nonce))] = exp | |
| 120 | + | s.links[sha256.Sum256([]byte(nonce))] = pendingLink{expires: exp, sessionCap: p.ExpiresAt()} | |
| 111 | 121 | s.mu.Unlock() | |
| 112 | 122 | ||
| 113 | 123 | log := zerowrap.FromCtx(ctx) | |
| @@ -121,24 +131,26 @@ func (s *Sessions) LoginLinkValid(nonce string) bool { | |||
| 121 | 131 | h := sha256.Sum256([]byte(nonce)) | |
| 122 | 132 | s.mu.Lock() | |
| 123 | 133 | defer s.mu.Unlock() | |
| 124 | - | exp, ok := s.links[h] | |
| 125 | - | return ok && s.clock.Now().Before(exp) | |
| 134 | + | l, ok := s.links[h] | |
| 135 | + | return ok && s.clock.Now().Before(l.expires) | |
| 126 | 136 | } | |
| 127 | 137 | ||
| 128 | 138 | // Login consumes the link and opens a session: it returns the signed session | |
| 129 | 139 | // token, for the cookie, and when it expires. A link that is unknown, expired | |
| 130 | 140 | // or already used gives domain.ErrUnauthenticated, whatever the reason. The | |
| 131 | - | // nonce and the token are never logged. | |
| 141 | + | // nonce and the token are never logged. The session lasts SessionTTL, or until | |
| 142 | + | // the expiry of the token that created the link when that comes first; a link | |
| 143 | + | // whose token has expired since gives domain.ErrUnauthenticated. | |
| 132 | 144 | func (s *Sessions) Login(ctx context.Context, nonce string) (token string, expiresAt time.Time, err error) { | |
| 133 | 145 | h := sha256.Sum256([]byte(nonce)) | |
| 134 | 146 | s.mu.Lock() | |
| 135 | - | exp, ok := s.links[h] | |
| 147 | + | l, ok := s.links[h] | |
| 136 | 148 | delete(s.links, h) // single use, also when expired | |
| 137 | 149 | s.mu.Unlock() | |
| 138 | - | if !ok || !s.clock.Now().Before(exp) { | |
| 150 | + | if !ok || !s.clock.Now().Before(l.expires) { | |
| 139 | 151 | return "", time.Time{}, domain.ErrUnauthenticated | |
| 140 | 152 | } | |
| 141 | - | token, expiresAt, err = s.issue(ctx) | |
| 153 | + | token, expiresAt, err = s.issue(ctx, l.sessionCap) | |
| 142 | 154 | if err != nil { | |
| 143 | 155 | return "", time.Time{}, err | |
| 144 | 156 | } | |
| @@ -154,13 +166,20 @@ type sessionClaims struct { | |||
| 154 | 166 | jwt.RegisteredClaims | |
| 155 | 167 | } | |
| 156 | 168 | ||
| 157 | - | func (s *Sessions) issue(ctx context.Context) (string, time.Time, error) { | |
| 169 | + | // issue signs a session for SessionTTL, cut at capAt unless that is zero. | |
| 170 | + | func (s *Sessions) issue(ctx context.Context, capAt time.Time) (string, time.Time, error) { | |
| 158 | 171 | epoch, err := s.store.Epoch(ctx) | |
| 159 | 172 | if err != nil { | |
| 160 | 173 | return "", time.Time{}, fmt.Errorf("read session epoch: %w", err) | |
| 161 | 174 | } | |
| 162 | 175 | now := s.clock.Now().UTC() | |
| 163 | 176 | exp := now.Add(SessionTTL) | |
| 177 | + | if !capAt.IsZero() && capAt.Before(exp) { | |
| 178 | + | exp = capAt.UTC() | |
| 179 | + | } | |
| 180 | + | if !exp.Truncate(time.Second).After(now) { | |
| 181 | + | return "", time.Time{}, domain.ErrUnauthenticated | |
| 182 | + | } | |
| 164 | 183 | tok, err := jwt.NewWithClaims(jwt.SigningMethodHS256, sessionClaims{ | |
| 165 | 184 | Epoch: &epoch, | |
| 166 | 185 | RegisteredClaims: jwt.RegisteredClaims{ | |
| @@ -56,7 +56,15 @@ type Sessions struct { | |||||
| 56 | key []byte | 56 | key []byte | ||
| 57 | 57 | ||||
| 58 | mu sync.Mutex | 58 | mu sync.Mutex | ||
| 59 | - | links map[[sha256.Size]byte]time.Time // by hash of the nonce, to the expiry | 59 | + | links map[[sha256.Size]byte]pendingLink // by hash of the nonce |
| 60 | + | } | |||
| 61 | + | ||||
| 62 | + | // pendingLink is a login link that waits to be used. | |||
| 63 | + | type pendingLink struct { | |||
| 64 | + | expires time.Time | |||
| 65 | + | // sessionCap is the latest moment the session it opens may last until: the | |||
| 66 | + | // expiry of the token that created the link. Zero means no cap. | |||
| 67 | + | sessionCap time.Time | |||
| 60 | } | 68 | } | ||
| 61 | 69 | ||||
| 62 | // NewSessions builds the session use cases and loads the signing key, which | 70 | // NewSessions builds the session use cases and loads the signing key, which | ||
| @@ -67,7 +75,7 @@ func NewSessions(ctx context.Context, store ports.SessionStore, clock ports.Cloc | |||||
| 67 | if err != nil { | 75 | if err != nil { | ||
| 68 | return nil, fmt.Errorf("load session key: %w", err) | 76 | return nil, fmt.Errorf("load session key: %w", err) | ||
| 69 | } | 77 | } | ||
| 70 | - | return &Sessions{store: store, clock: clock, publicURL: publicURL, key: key, links: map[[sha256.Size]byte]time.Time{}}, nil | 78 | + | return &Sessions{store: store, clock: clock, publicURL: publicURL, key: key, links: map[[sha256.Size]byte]pendingLink{}}, nil |
| 71 | } | 79 | } | ||
| 72 | 80 | ||||
| 73 | // requireOwner is the access rule of the session API: root, or a stored token | 81 | // requireOwner is the access rule of the session API: root, or a stored token | ||
| @@ -83,7 +91,9 @@ func requireOwner(p domain.Principal) error { | |||||
| 83 | } | 91 | } | ||
| 84 | 92 | ||||
| 85 | // CreateLoginLink makes a link valid for LoginLinkTTL and usable once. Only | 93 | // CreateLoginLink makes a link valid for LoginLinkTTL and usable once. Only | ||
| 86 | - | // the owner may. It gives domain.ErrResourceExhausted when MaxPendingLoginLinks | 94 | + | // the owner may. The session the link opens never outlives the token that |
| 95 | + | // created it: it is capped at that token's expiry (no cap for root, or a token | |||
| 96 | + | // that never expires). It gives domain.ErrResourceExhausted when MaxPendingLoginLinks | |||
| 87 | // links wait. The nonce is never logged. | 97 | // links wait. The nonce is never logged. | ||
| 88 | func (s *Sessions) CreateLoginLink(ctx context.Context, p domain.Principal) (LoginLink, error) { | 98 | func (s *Sessions) CreateLoginLink(ctx context.Context, p domain.Principal) (LoginLink, error) { | ||
| 89 | if err := requireOwner(p); err != nil { | 99 | if err := requireOwner(p); err != nil { | ||
| @@ -98,8 +108,8 @@ func (s *Sessions) CreateLoginLink(ctx context.Context, p domain.Principal) (Log | |||||
| 98 | exp := now.Add(LoginLinkTTL) | 108 | exp := now.Add(LoginLinkTTL) | ||
| 99 | 109 | ||||
| 100 | s.mu.Lock() | 110 | s.mu.Lock() | ||
| 101 | - | for h, e := range s.links { | 111 | + | for h, l := range s.links { |
| 102 | - | if !now.Before(e) { | 112 | + | if !now.Before(l.expires) { |
| 103 | delete(s.links, h) | 113 | delete(s.links, h) | ||
| 104 | } | 114 | } | ||
| 105 | } | 115 | } | ||
| @@ -107,7 +117,7 @@ func (s *Sessions) CreateLoginLink(ctx context.Context, p domain.Principal) (Log | |||||
| 107 | s.mu.Unlock() | 117 | s.mu.Unlock() | ||
| 108 | return LoginLink{}, fmt.Errorf("%w: %d login links are already pending, use one or wait a minute", domain.ErrResourceExhausted, MaxPendingLoginLinks) | 118 | return LoginLink{}, fmt.Errorf("%w: %d login links are already pending, use one or wait a minute", domain.ErrResourceExhausted, MaxPendingLoginLinks) | ||
| 109 | } | 119 | } | ||
| 110 | - | s.links[sha256.Sum256([]byte(nonce))] = exp | 120 | + | s.links[sha256.Sum256([]byte(nonce))] = pendingLink{expires: exp, sessionCap: p.ExpiresAt()} |
| 111 | s.mu.Unlock() | 121 | s.mu.Unlock() | ||
| 112 | 122 | ||||
| 113 | log := zerowrap.FromCtx(ctx) | 123 | log := zerowrap.FromCtx(ctx) | ||
| @@ -121,24 +131,26 @@ func (s *Sessions) LoginLinkValid(nonce string) bool { | |||||
| 121 | h := sha256.Sum256([]byte(nonce)) | 131 | h := sha256.Sum256([]byte(nonce)) | ||
| 122 | s.mu.Lock() | 132 | s.mu.Lock() | ||
| 123 | defer s.mu.Unlock() | 133 | defer s.mu.Unlock() | ||
| 124 | - | exp, ok := s.links[h] | 134 | + | l, ok := s.links[h] |
| 125 | - | return ok && s.clock.Now().Before(exp) | 135 | + | return ok && s.clock.Now().Before(l.expires) |
| 126 | } | 136 | } | ||
| 127 | 137 | ||||
| 128 | // Login consumes the link and opens a session: it returns the signed session | 138 | // Login consumes the link and opens a session: it returns the signed session | ||
| 129 | // token, for the cookie, and when it expires. A link that is unknown, expired | 139 | // token, for the cookie, and when it expires. A link that is unknown, expired | ||
| 130 | // or already used gives domain.ErrUnauthenticated, whatever the reason. The | 140 | // or already used gives domain.ErrUnauthenticated, whatever the reason. The | ||
| 131 | - | // nonce and the token are never logged. | 141 | + | // nonce and the token are never logged. The session lasts SessionTTL, or until |
| 142 | + | // the expiry of the token that created the link when that comes first; a link | |||
| 143 | + | // whose token has expired since gives domain.ErrUnauthenticated. | |||
| 132 | func (s *Sessions) Login(ctx context.Context, nonce string) (token string, expiresAt time.Time, err error) { | 144 | func (s *Sessions) Login(ctx context.Context, nonce string) (token string, expiresAt time.Time, err error) { | ||
| 133 | h := sha256.Sum256([]byte(nonce)) | 145 | h := sha256.Sum256([]byte(nonce)) | ||
| 134 | s.mu.Lock() | 146 | s.mu.Lock() | ||
| 135 | - | exp, ok := s.links[h] | 147 | + | l, ok := s.links[h] |
| 136 | delete(s.links, h) // single use, also when expired | 148 | delete(s.links, h) // single use, also when expired | ||
| 137 | s.mu.Unlock() | 149 | s.mu.Unlock() | ||
| 138 | - | if !ok || !s.clock.Now().Before(exp) { | 150 | + | if !ok || !s.clock.Now().Before(l.expires) { |
| 139 | return "", time.Time{}, domain.ErrUnauthenticated | 151 | return "", time.Time{}, domain.ErrUnauthenticated | ||
| 140 | } | 152 | } | ||
| 141 | - | token, expiresAt, err = s.issue(ctx) | 153 | + | token, expiresAt, err = s.issue(ctx, l.sessionCap) |
| 142 | if err != nil { | 154 | if err != nil { | ||
| 143 | return "", time.Time{}, err | 155 | return "", time.Time{}, err | ||
| 144 | } | 156 | } | ||
| @@ -154,13 +166,20 @@ type sessionClaims struct { | |||||
| 154 | jwt.RegisteredClaims | 166 | jwt.RegisteredClaims | ||
| 155 | } | 167 | } | ||
| 156 | 168 | ||||
| 157 | - | func (s *Sessions) issue(ctx context.Context) (string, time.Time, error) { | 169 | + | // issue signs a session for SessionTTL, cut at capAt unless that is zero. |
| 170 | + | func (s *Sessions) issue(ctx context.Context, capAt time.Time) (string, time.Time, error) { | |||
| 158 | epoch, err := s.store.Epoch(ctx) | 171 | epoch, err := s.store.Epoch(ctx) | ||
| 159 | if err != nil { | 172 | if err != nil { | ||
| 160 | return "", time.Time{}, fmt.Errorf("read session epoch: %w", err) | 173 | return "", time.Time{}, fmt.Errorf("read session epoch: %w", err) | ||
| 161 | } | 174 | } | ||
| 162 | now := s.clock.Now().UTC() | 175 | now := s.clock.Now().UTC() | ||
| 163 | exp := now.Add(SessionTTL) | 176 | exp := now.Add(SessionTTL) | ||
| 177 | + | if !capAt.IsZero() && capAt.Before(exp) { | |||
| 178 | + | exp = capAt.UTC() | |||
| 179 | + | } | |||
| 180 | + | if !exp.Truncate(time.Second).After(now) { | |||
| 181 | + | return "", time.Time{}, domain.ErrUnauthenticated | |||
| 182 | + | } | |||
| 164 | tok, err := jwt.NewWithClaims(jwt.SigningMethodHS256, sessionClaims{ | 183 | tok, err := jwt.NewWithClaims(jwt.SigningMethodHS256, sessionClaims{ | ||
| 165 | Epoch: &epoch, | 184 | Epoch: &epoch, | ||
| 166 | RegisteredClaims: jwt.RegisteredClaims{ | 185 | RegisteredClaims: jwt.RegisteredClaims{ | ||
M internal/app/sessions_test.go +56 -0
| @@ -172,6 +172,62 @@ func TestLoginLinkLifecycle(t *testing.T) { | |||
| 172 | 172 | assert.True(t, f.sessions.LoginLinkValid(b)) | |
| 173 | 173 | } | |
| 174 | 174 | ||
| 175 | + | // GO-003a: the session of a link made with a token never outlives that token; | |
| 176 | + | // root and a token that never expires leave it at SessionTTL. | |
| 177 | + | func TestSessionIsCappedByTheTokenThatMadeTheLink(t *testing.T) { | |
| 178 | + | ctx := context.Background() | |
| 179 | + | f := newSessionFixture(t) | |
| 180 | + | tokenExpiring := func(in time.Duration) domain.Principal { | |
| 181 | + | return domain.TokenPrincipal(domain.Token{ID: "t", Scopes: domain.AllScopes(), ExpiresAt: f.clock.Now().Add(in)}) | |
| 182 | + | } | |
| 183 | + | login := func(p domain.Principal) (string, time.Time, error) { | |
| 184 | + | link, err := f.sessions.CreateLoginLink(ctx, p) | |
| 185 | + | require.NoError(t, err) | |
| 186 | + | return f.sessions.Login(ctx, strings.TrimPrefix(link.URL, "https://forge.example/-/login/")) | |
| 187 | + | } | |
| 188 | + | start := f.clock.Now() | |
| 189 | + | ||
| 190 | + | for name, tc := range map[string]struct { | |
| 191 | + | p domain.Principal | |
| 192 | + | want time.Time | |
| 193 | + | }{ | |
| 194 | + | "root": {domain.Root(), start.Add(app.SessionTTL)}, | |
| 195 | + | "owner session": {domain.Owner(), start.Add(app.SessionTTL)}, | |
| 196 | + | "never expires": {allScopeToken(), start.Add(app.SessionTTL)}, | |
| 197 | + | "expires sooner": {tokenExpiring(2 * time.Hour), start.Add(2 * time.Hour)}, | |
| 198 | + | "expires later": {tokenExpiring(48 * time.Hour), start.Add(app.SessionTTL)}, | |
| 199 | + | "expires exactly TTL": {tokenExpiring(app.SessionTTL), start.Add(app.SessionTTL)}, | |
| 200 | + | } { | |
| 201 | + | tok, exp, err := login(tc.p) | |
| 202 | + | require.NoError(t, err, name) | |
| 203 | + | assert.Equal(t, tc.want, exp, name) | |
| 204 | + | _, err = f.sessions.Verify(ctx, tok) | |
| 205 | + | assert.NoError(t, err, name) | |
| 206 | + | // The session token itself carries the cap as its exp claim. | |
| 207 | + | var c claims | |
| 208 | + | _, _, err = jwt.NewParser().ParseUnverified(tok, &c) | |
| 209 | + | require.NoError(t, err, name) | |
| 210 | + | assert.Equal(t, tc.want, c.ExpiresAt.UTC(), name) | |
| 211 | + | } | |
| 212 | + | ||
| 213 | + | // It stops working when the token would have. | |
| 214 | + | tok, _, err := login(tokenExpiring(2 * time.Hour)) | |
| 215 | + | require.NoError(t, err) | |
| 216 | + | f.clock.advance(2*time.Hour - time.Second) | |
| 217 | + | _, err = f.sessions.Verify(ctx, tok) | |
| 218 | + | assert.NoError(t, err) | |
| 219 | + | f.clock.advance(time.Second) | |
| 220 | + | _, err = f.sessions.Verify(ctx, tok) | |
| 221 | + | assert.ErrorIs(t, err, domain.ErrUnauthenticated, "the token expired, so did the session") | |
| 222 | + | ||
| 223 | + | // A token that expires before the link is used opens nothing. | |
| 224 | + | link, err := f.sessions.CreateLoginLink(ctx, tokenExpiring(30*time.Second)) | |
| 225 | + | require.NoError(t, err) | |
| 226 | + | f.clock.advance(31 * time.Second) | |
| 227 | + | _, _, err = f.sessions.Login(ctx, strings.TrimPrefix(link.URL, "https://forge.example/-/login/")) | |
| 228 | + | assert.ErrorIs(t, err, domain.ErrUnauthenticated) | |
| 229 | + | } | |
| 230 | + | ||
| 175 | 231 | // A single use also holds under concurrent confirmations. | |
| 176 | 232 | func TestLoginLinkConsumedOnceConcurrently(t *testing.T) { | |
| 177 | 233 | f := newSessionFixture(t) | |
| @@ -172,6 +172,62 @@ func TestLoginLinkLifecycle(t *testing.T) { | |||||
| 172 | assert.True(t, f.sessions.LoginLinkValid(b)) | 172 | assert.True(t, f.sessions.LoginLinkValid(b)) | ||
| 173 | } | 173 | } | ||
| 174 | 174 | ||||
| 175 | + | // GO-003a: the session of a link made with a token never outlives that token; | |||
| 176 | + | // root and a token that never expires leave it at SessionTTL. | |||
| 177 | + | func TestSessionIsCappedByTheTokenThatMadeTheLink(t *testing.T) { | |||
| 178 | + | ctx := context.Background() | |||
| 179 | + | f := newSessionFixture(t) | |||
| 180 | + | tokenExpiring := func(in time.Duration) domain.Principal { | |||
| 181 | + | return domain.TokenPrincipal(domain.Token{ID: "t", Scopes: domain.AllScopes(), ExpiresAt: f.clock.Now().Add(in)}) | |||
| 182 | + | } | |||
| 183 | + | login := func(p domain.Principal) (string, time.Time, error) { | |||
| 184 | + | link, err := f.sessions.CreateLoginLink(ctx, p) | |||
| 185 | + | require.NoError(t, err) | |||
| 186 | + | return f.sessions.Login(ctx, strings.TrimPrefix(link.URL, "https://forge.example/-/login/")) | |||
| 187 | + | } | |||
| 188 | + | start := f.clock.Now() | |||
| 189 | + | ||||
| 190 | + | for name, tc := range map[string]struct { | |||
| 191 | + | p domain.Principal | |||
| 192 | + | want time.Time | |||
| 193 | + | }{ | |||
| 194 | + | "root": {domain.Root(), start.Add(app.SessionTTL)}, | |||
| 195 | + | "owner session": {domain.Owner(), start.Add(app.SessionTTL)}, | |||
| 196 | + | "never expires": {allScopeToken(), start.Add(app.SessionTTL)}, | |||
| 197 | + | "expires sooner": {tokenExpiring(2 * time.Hour), start.Add(2 * time.Hour)}, | |||
| 198 | + | "expires later": {tokenExpiring(48 * time.Hour), start.Add(app.SessionTTL)}, | |||
| 199 | + | "expires exactly TTL": {tokenExpiring(app.SessionTTL), start.Add(app.SessionTTL)}, | |||
| 200 | + | } { | |||
| 201 | + | tok, exp, err := login(tc.p) | |||
| 202 | + | require.NoError(t, err, name) | |||
| 203 | + | assert.Equal(t, tc.want, exp, name) | |||
| 204 | + | _, err = f.sessions.Verify(ctx, tok) | |||
| 205 | + | assert.NoError(t, err, name) | |||
| 206 | + | // The session token itself carries the cap as its exp claim. | |||
| 207 | + | var c claims | |||
| 208 | + | _, _, err = jwt.NewParser().ParseUnverified(tok, &c) | |||
| 209 | + | require.NoError(t, err, name) | |||
| 210 | + | assert.Equal(t, tc.want, c.ExpiresAt.UTC(), name) | |||
| 211 | + | } | |||
| 212 | + | ||||
| 213 | + | // It stops working when the token would have. | |||
| 214 | + | tok, _, err := login(tokenExpiring(2 * time.Hour)) | |||
| 215 | + | require.NoError(t, err) | |||
| 216 | + | f.clock.advance(2*time.Hour - time.Second) | |||
| 217 | + | _, err = f.sessions.Verify(ctx, tok) | |||
| 218 | + | assert.NoError(t, err) | |||
| 219 | + | f.clock.advance(time.Second) | |||
| 220 | + | _, err = f.sessions.Verify(ctx, tok) | |||
| 221 | + | assert.ErrorIs(t, err, domain.ErrUnauthenticated, "the token expired, so did the session") | |||
| 222 | + | ||||
| 223 | + | // A token that expires before the link is used opens nothing. | |||
| 224 | + | link, err := f.sessions.CreateLoginLink(ctx, tokenExpiring(30*time.Second)) | |||
| 225 | + | require.NoError(t, err) | |||
| 226 | + | f.clock.advance(31 * time.Second) | |||
| 227 | + | _, _, err = f.sessions.Login(ctx, strings.TrimPrefix(link.URL, "https://forge.example/-/login/")) | |||
| 228 | + | assert.ErrorIs(t, err, domain.ErrUnauthenticated) | |||
| 229 | + | } | |||
| 230 | + | ||||
| 175 | // A single use also holds under concurrent confirmations. | 231 | // A single use also holds under concurrent confirmations. | ||
| 176 | func TestLoginLinkConsumedOnceConcurrently(t *testing.T) { | 232 | func TestLoginLinkConsumedOnceConcurrently(t *testing.T) { | ||
| 177 | f := newSessionFixture(t) | 233 | f := newSessionFixture(t) | ||
M internal/app/tokens.go +27 -13
| @@ -29,13 +29,15 @@ const touchInterval = time.Minute | |||
| 29 | 29 | ||
| 30 | 30 | // Tokens are the use cases that manage stored tokens. | |
| 31 | 31 | type Tokens struct { | |
| 32 | - | store ports.TokenStore | |
| 33 | - | clock ports.Clock | |
| 32 | + | store ports.TokenStore | |
| 33 | + | clock ports.Clock | |
| 34 | + | sessions ports.SessionStore | |
| 34 | 35 | } | |
| 35 | 36 | ||
| 36 | - | // NewTokens builds the token use cases over their ports. | |
| 37 | - | func NewTokens(store ports.TokenStore, clock ports.Clock) *Tokens { | |
| 38 | - | return &Tokens{store: store, clock: clock} | |
| 37 | + | // NewTokens builds the token use cases over their ports. sessions is where | |
| 38 | + | // revoking a token with owner rights ends every web session (see Revoke). | |
| 39 | + | func NewTokens(store ports.TokenStore, clock ports.Clock, sessions ports.SessionStore) *Tokens { | |
| 40 | + | return &Tokens{store: store, clock: clock, sessions: sessions} | |
| 39 | 41 | } | |
| 40 | 42 | ||
| 41 | 43 | // CreateTokenParams are the settings of a new token. | |
| @@ -194,24 +196,36 @@ func (t *Tokens) List(ctx context.Context, p domain.Principal) ([]domain.Token, | |||
| 194 | 196 | // token:admin and returns domain.ErrNotFound for an unknown id, and also for | |
| 195 | 197 | // a token more powerful than the principal (the no-escalation rule of | |
| 196 | 198 | // Create), so that it does not show itself. Root may revoke any token. | |
| 199 | + | // | |
| 200 | + | // A token that has owner rights (HasOwnerRights: every scope, no repository | |
| 201 | + | // list) could open web sessions, so revoking it also signs every browser out, | |
| 202 | + | // as `ermite web logout --all` does: web sessions are not tied to the token | |
| 203 | + | // that opened them. The revocation holds even if signing out fails, which is | |
| 204 | + | // then reported as an error. | |
| 197 | 205 | func (t *Tokens) Revoke(ctx context.Context, p domain.Principal, id string) error { | |
| 198 | 206 | if err := requireTokenAdmin(p); err != nil { | |
| 199 | 207 | return err | |
| 200 | 208 | } | |
| 201 | - | if !p.IsRoot() { | |
| 202 | - | tok, err := t.store.Get(ctx, id) | |
| 203 | - | if err != nil { | |
| 204 | - | return err | |
| 205 | - | } | |
| 206 | - | if checkNoEscalation(p, tok) != nil { | |
| 207 | - | return domain.ErrNotFound | |
| 208 | - | } | |
| 209 | + | tok, err := t.store.Get(ctx, id) | |
| 210 | + | if err != nil { | |
| 211 | + | return err | |
| 212 | + | } | |
| 213 | + | if !p.IsRoot() && checkNoEscalation(p, tok) != nil { | |
| 214 | + | return domain.ErrNotFound | |
| 209 | 215 | } | |
| 210 | 216 | if err := t.store.Revoke(ctx, id); err != nil { | |
| 211 | 217 | return err | |
| 212 | 218 | } | |
| 213 | 219 | log := zerowrap.FromCtx(ctx) | |
| 214 | 220 | log.Info().Str("token_id", id).Str("revoked_by", p.Label()).Str("revoked_by_token", p.TokenID()).Msg("token revoked") | |
| 221 | + | if !domain.TokenPrincipal(tok).HasOwnerRights() { | |
| 222 | + | return nil | |
| 223 | + | } | |
| 224 | + | epoch, err := t.sessions.BumpEpoch(ctx) | |
| 225 | + | if err != nil { | |
| 226 | + | return fmt.Errorf("token %s revoked, but signing out the web sessions failed: %w", id, err) | |
| 227 | + | } | |
| 228 | + | log.Info().Str("token_id", id).Uint64("epoch", epoch).Msg("every web session signed out: the revoked token had owner rights") | |
| 215 | 229 | return nil | |
| 216 | 230 | } | |
| 217 | 231 | ||
| @@ -29,13 +29,15 @@ const touchInterval = time.Minute | |||||
| 29 | 29 | ||||
| 30 | // Tokens are the use cases that manage stored tokens. | 30 | // Tokens are the use cases that manage stored tokens. | ||
| 31 | type Tokens struct { | 31 | type Tokens struct { | ||
| 32 | - | store ports.TokenStore | 32 | + | store ports.TokenStore |
| 33 | - | clock ports.Clock | 33 | + | clock ports.Clock |
| 34 | + | sessions ports.SessionStore | |||
| 34 | } | 35 | } | ||
| 35 | 36 | ||||
| 36 | - | // NewTokens builds the token use cases over their ports. | 37 | + | // NewTokens builds the token use cases over their ports. sessions is where |
| 37 | - | func NewTokens(store ports.TokenStore, clock ports.Clock) *Tokens { | 38 | + | // revoking a token with owner rights ends every web session (see Revoke). |
| 38 | - | return &Tokens{store: store, clock: clock} | 39 | + | func NewTokens(store ports.TokenStore, clock ports.Clock, sessions ports.SessionStore) *Tokens { |
| 40 | + | return &Tokens{store: store, clock: clock, sessions: sessions} | |||
| 39 | } | 41 | } | ||
| 40 | 42 | ||||
| 41 | // CreateTokenParams are the settings of a new token. | 43 | // CreateTokenParams are the settings of a new token. | ||
| @@ -194,24 +196,36 @@ func (t *Tokens) List(ctx context.Context, p domain.Principal) ([]domain.Token, | |||||
| 194 | // token:admin and returns domain.ErrNotFound for an unknown id, and also for | 196 | // token:admin and returns domain.ErrNotFound for an unknown id, and also for | ||
| 195 | // a token more powerful than the principal (the no-escalation rule of | 197 | // a token more powerful than the principal (the no-escalation rule of | ||
| 196 | // Create), so that it does not show itself. Root may revoke any token. | 198 | // Create), so that it does not show itself. Root may revoke any token. | ||
| 199 | + | // | |||
| 200 | + | // A token that has owner rights (HasOwnerRights: every scope, no repository | |||
| 201 | + | // list) could open web sessions, so revoking it also signs every browser out, | |||
| 202 | + | // as `ermite web logout --all` does: web sessions are not tied to the token | |||
| 203 | + | // that opened them. The revocation holds even if signing out fails, which is | |||
| 204 | + | // then reported as an error. | |||
| 197 | func (t *Tokens) Revoke(ctx context.Context, p domain.Principal, id string) error { | 205 | func (t *Tokens) Revoke(ctx context.Context, p domain.Principal, id string) error { | ||
| 198 | if err := requireTokenAdmin(p); err != nil { | 206 | if err := requireTokenAdmin(p); err != nil { | ||
| 199 | return err | 207 | return err | ||
| 200 | } | 208 | } | ||
| 201 | - | if !p.IsRoot() { | 209 | + | tok, err := t.store.Get(ctx, id) |
| 202 | - | tok, err := t.store.Get(ctx, id) | 210 | + | if err != nil { |
| 203 | - | if err != nil { | 211 | + | return err |
| 204 | - | return err | 212 | + | } |
| 205 | - | } | 213 | + | if !p.IsRoot() && checkNoEscalation(p, tok) != nil { |
| 206 | - | if checkNoEscalation(p, tok) != nil { | 214 | + | return domain.ErrNotFound |
| 207 | - | return domain.ErrNotFound | |||
| 208 | - | } | |||
| 209 | } | 215 | } | ||
| 210 | if err := t.store.Revoke(ctx, id); err != nil { | 216 | if err := t.store.Revoke(ctx, id); err != nil { | ||
| 211 | return err | 217 | return err | ||
| 212 | } | 218 | } | ||
| 213 | log := zerowrap.FromCtx(ctx) | 219 | log := zerowrap.FromCtx(ctx) | ||
| 214 | log.Info().Str("token_id", id).Str("revoked_by", p.Label()).Str("revoked_by_token", p.TokenID()).Msg("token revoked") | 220 | log.Info().Str("token_id", id).Str("revoked_by", p.Label()).Str("revoked_by_token", p.TokenID()).Msg("token revoked") | ||
| 221 | + | if !domain.TokenPrincipal(tok).HasOwnerRights() { | |||
| 222 | + | return nil | |||
| 223 | + | } | |||
| 224 | + | epoch, err := t.sessions.BumpEpoch(ctx) | |||
| 225 | + | if err != nil { | |||
| 226 | + | return fmt.Errorf("token %s revoked, but signing out the web sessions failed: %w", id, err) | |||
| 227 | + | } | |||
| 228 | + | log.Info().Str("token_id", id).Uint64("epoch", epoch).Msg("every web session signed out: the revoked token had owner rights") | |||
| 215 | return nil | 229 | return nil | ||
| 216 | } | 230 | } | ||
| 217 | 231 | ||||
M internal/app/tokens_test.go +62 -9
| @@ -20,19 +20,20 @@ import ( | |||
| 20 | 20 | ) | |
| 21 | 21 | ||
| 22 | 22 | type tokenFixture struct { | |
| 23 | - | store *mocks.TokenStore | |
| 24 | - | clock *mocks.Clock | |
| 25 | - | tokens *app.Tokens | |
| 26 | - | auth *app.Auth | |
| 23 | + | store *mocks.TokenStore | |
| 24 | + | clock *mocks.Clock | |
| 25 | + | sessions *mocks.SessionStore | |
| 26 | + | tokens *app.Tokens | |
| 27 | + | auth *app.Auth | |
| 27 | 28 | } | |
| 28 | 29 | ||
| 29 | 30 | var rootSecret = "root-secret-0123456789abcdef0123456789" | |
| 30 | 31 | ||
| 31 | 32 | func newTokenFixture(t *testing.T) *tokenFixture { | |
| 32 | 33 | t.Helper() | |
| 33 | - | f := &tokenFixture{store: mocks.NewTokenStore(t), clock: mocks.NewClock(t)} | |
| 34 | + | f := &tokenFixture{store: mocks.NewTokenStore(t), clock: mocks.NewClock(t), sessions: mocks.NewSessionStore(t)} | |
| 34 | 35 | f.clock.EXPECT().Now().Return(now).Maybe() | |
| 35 | - | f.tokens = app.NewTokens(f.store, f.clock) | |
| 36 | + | f.tokens = app.NewTokens(f.store, f.clock, f.sessions) | |
| 36 | 37 | f.auth = app.NewAuth(domain.HashTokenSecret(rootSecret), f.store, f.clock) | |
| 37 | 38 | return f | |
| 38 | 39 | } | |
| @@ -216,9 +217,9 @@ func TestTokensListRevoke(t *testing.T) { | |||
| 216 | 217 | ||
| 217 | 218 | t.Run("revoke", func(t *testing.T) { | |
| 218 | 219 | f := newTokenFixture(t) | |
| 219 | - | f.store.EXPECT().Get(ctx, "a").Return(domain.Token{ID: "a", Scopes: scopes(domain.ScopeTokenAdmin)}, nil).Once() | |
| 220 | + | f.store.EXPECT().Get(ctx, "a").Return(domain.Token{ID: "a", Scopes: scopes(domain.ScopeTokenAdmin)}, nil).Twice() | |
| 221 | + | f.store.EXPECT().Get(ctx, "zz").Return(domain.Token{}, domain.ErrNotFound).Once() | |
| 220 | 222 | f.store.EXPECT().Revoke(ctx, "a").Return(nil).Twice() | |
| 221 | - | f.store.EXPECT().Revoke(ctx, "zz").Return(domain.ErrNotFound).Once() | |
| 222 | 223 | require.NoError(t, f.tokens.Revoke(ctx, tokenAdmin, "a")) | |
| 223 | 224 | require.NoError(t, f.tokens.Revoke(ctx, domain.Root(), "a")) | |
| 224 | 225 | assert.ErrorIs(t, f.tokens.Revoke(ctx, domain.Root(), "zz"), domain.ErrNotFound) | |
| @@ -227,6 +228,56 @@ func TestTokensListRevoke(t *testing.T) { | |||
| 227 | 228 | }) | |
| 228 | 229 | } | |
| 229 | 230 | ||
| 231 | + | // GO-003b: revoking a token with owner rights signs every browser out (the | |
| 232 | + | // session epoch is bumped); revoking any other token does not. | |
| 233 | + | func TestTokensRevokeOwnerRightsEndsWebSessions(t *testing.T) { | |
| 234 | + | ctx := context.Background() | |
| 235 | + | ownerTok := domain.Token{ID: "own", Scopes: domain.AllScopes()} | |
| 236 | + | ||
| 237 | + | for name, tc := range map[string]struct { | |
| 238 | + | tok domain.Token | |
| 239 | + | by domain.Principal | |
| 240 | + | bumped bool | |
| 241 | + | }{ | |
| 242 | + | "every scope, by root": {ownerTok, domain.Root(), true}, | |
| 243 | + | "every scope, by an owner key": {ownerTok, domain.TokenPrincipal(domain.Token{ID: "adm", Scopes: domain.AllScopes()}), true}, | |
| 244 | + | "one scope short": {domain.Token{ID: "p", Scopes: domain.AllScopes()[1:]}, domain.Root(), false}, | |
| 245 | + | "repo list": {domain.Token{ID: "l", Scopes: domain.AllScopes(), Repos: names(t, "a")}, domain.Root(), false}, | |
| 246 | + | } { | |
| 247 | + | t.Run(name, func(t *testing.T) { | |
| 248 | + | f := newTokenFixture(t) | |
| 249 | + | f.store.EXPECT().Get(ctx, tc.tok.ID).Return(tc.tok, nil) | |
| 250 | + | f.store.EXPECT().Revoke(ctx, tc.tok.ID).Return(nil) | |
| 251 | + | if tc.bumped { | |
| 252 | + | f.sessions.EXPECT().BumpEpoch(ctx).Return(1, nil).Once() | |
| 253 | + | } | |
| 254 | + | require.NoError(t, f.tokens.Revoke(ctx, tc.by, tc.tok.ID)) | |
| 255 | + | }) | |
| 256 | + | } | |
| 257 | + | ||
| 258 | + | t.Run("a revoke that fails bumps nothing", func(t *testing.T) { | |
| 259 | + | f := newTokenFixture(t) | |
| 260 | + | f.store.EXPECT().Get(ctx, "own").Return(ownerTok, nil) | |
| 261 | + | f.store.EXPECT().Revoke(ctx, "own").Return(errors.New("kv down")) | |
| 262 | + | assert.Error(t, f.tokens.Revoke(ctx, domain.Root(), "own")) | |
| 263 | + | }) | |
| 264 | + | ||
| 265 | + | t.Run("a token out of reach is not revoked and bumps nothing", func(t *testing.T) { | |
| 266 | + | f := newTokenFixture(t) | |
| 267 | + | limited := domain.TokenPrincipal(domain.Token{ID: "adm", Scopes: scopes(domain.ScopeTokenAdmin)}) | |
| 268 | + | f.store.EXPECT().Get(ctx, "own").Return(ownerTok, nil) | |
| 269 | + | assert.ErrorIs(t, f.tokens.Revoke(ctx, limited, "own"), domain.ErrNotFound) | |
| 270 | + | }) | |
| 271 | + | ||
| 272 | + | t.Run("a failure to sign out is reported once the token is revoked", func(t *testing.T) { | |
| 273 | + | f := newTokenFixture(t) | |
| 274 | + | f.store.EXPECT().Get(ctx, "own").Return(ownerTok, nil) | |
| 275 | + | f.store.EXPECT().Revoke(ctx, "own").Return(nil) | |
| 276 | + | f.sessions.EXPECT().BumpEpoch(ctx).Return(0, errors.New("kv down")) | |
| 277 | + | assert.ErrorContains(t, f.tokens.Revoke(ctx, domain.Root(), "own"), "signing out") | |
| 278 | + | }) | |
| 279 | + | } | |
| 280 | + | ||
| 230 | 281 | // A token admin sees and revokes only the tokens it could have created: the | |
| 231 | 282 | // no-escalation rule of Create, applied to scopes, repositories and expiry. | |
| 232 | 283 | func TestTokensAdminReach(t *testing.T) { | |
| @@ -299,7 +350,9 @@ func TestTokensAdminReach(t *testing.T) { | |||
| 299 | 350 | got, err := f.tokens.List(ctx, domain.Root()) | |
| 300 | 351 | require.NoError(t, err) | |
| 301 | 352 | assert.Equal(t, all, got) | |
| 302 | - | f.store.EXPECT().Revoke(ctx, "x").Return(nil) // no Get: root has no limits | |
| 353 | + | f.store.EXPECT().Get(ctx, "x").Return(all[0], nil) | |
| 354 | + | f.store.EXPECT().Revoke(ctx, "x").Return(nil) // root has no limits | |
| 355 | + | f.sessions.EXPECT().BumpEpoch(ctx).Return(1, nil) // "x" has owner rights | |
| 303 | 356 | require.NoError(t, f.tokens.Revoke(ctx, domain.Root(), "x")) | |
| 304 | 357 | }) | |
| 305 | 358 | ||
| @@ -20,19 +20,20 @@ import ( | |||||
| 20 | ) | 20 | ) | ||
| 21 | 21 | ||||
| 22 | type tokenFixture struct { | 22 | type tokenFixture struct { | ||
| 23 | - | store *mocks.TokenStore | 23 | + | store *mocks.TokenStore |
| 24 | - | clock *mocks.Clock | 24 | + | clock *mocks.Clock |
| 25 | - | tokens *app.Tokens | 25 | + | sessions *mocks.SessionStore |
| 26 | - | auth *app.Auth | 26 | + | tokens *app.Tokens |
| 27 | + | auth *app.Auth | |||
| 27 | } | 28 | } | ||
| 28 | 29 | ||||
| 29 | var rootSecret = "root-secret-0123456789abcdef0123456789" | 30 | var rootSecret = "root-secret-0123456789abcdef0123456789" | ||
| 30 | 31 | ||||
| 31 | func newTokenFixture(t *testing.T) *tokenFixture { | 32 | func newTokenFixture(t *testing.T) *tokenFixture { | ||
| 32 | t.Helper() | 33 | t.Helper() | ||
| 33 | - | f := &tokenFixture{store: mocks.NewTokenStore(t), clock: mocks.NewClock(t)} | 34 | + | f := &tokenFixture{store: mocks.NewTokenStore(t), clock: mocks.NewClock(t), sessions: mocks.NewSessionStore(t)} |
| 34 | f.clock.EXPECT().Now().Return(now).Maybe() | 35 | f.clock.EXPECT().Now().Return(now).Maybe() | ||
| 35 | - | f.tokens = app.NewTokens(f.store, f.clock) | 36 | + | f.tokens = app.NewTokens(f.store, f.clock, f.sessions) |
| 36 | f.auth = app.NewAuth(domain.HashTokenSecret(rootSecret), f.store, f.clock) | 37 | f.auth = app.NewAuth(domain.HashTokenSecret(rootSecret), f.store, f.clock) | ||
| 37 | return f | 38 | return f | ||
| 38 | } | 39 | } | ||
| @@ -216,9 +217,9 @@ func TestTokensListRevoke(t *testing.T) { | |||||
| 216 | 217 | ||||
| 217 | t.Run("revoke", func(t *testing.T) { | 218 | t.Run("revoke", func(t *testing.T) { | ||
| 218 | f := newTokenFixture(t) | 219 | f := newTokenFixture(t) | ||
| 219 | - | f.store.EXPECT().Get(ctx, "a").Return(domain.Token{ID: "a", Scopes: scopes(domain.ScopeTokenAdmin)}, nil).Once() | 220 | + | f.store.EXPECT().Get(ctx, "a").Return(domain.Token{ID: "a", Scopes: scopes(domain.ScopeTokenAdmin)}, nil).Twice() |
| 221 | + | f.store.EXPECT().Get(ctx, "zz").Return(domain.Token{}, domain.ErrNotFound).Once() | |||
| 220 | f.store.EXPECT().Revoke(ctx, "a").Return(nil).Twice() | 222 | f.store.EXPECT().Revoke(ctx, "a").Return(nil).Twice() | ||
| 221 | - | f.store.EXPECT().Revoke(ctx, "zz").Return(domain.ErrNotFound).Once() | |||
| 222 | require.NoError(t, f.tokens.Revoke(ctx, tokenAdmin, "a")) | 223 | require.NoError(t, f.tokens.Revoke(ctx, tokenAdmin, "a")) | ||
| 223 | require.NoError(t, f.tokens.Revoke(ctx, domain.Root(), "a")) | 224 | require.NoError(t, f.tokens.Revoke(ctx, domain.Root(), "a")) | ||
| 224 | assert.ErrorIs(t, f.tokens.Revoke(ctx, domain.Root(), "zz"), domain.ErrNotFound) | 225 | assert.ErrorIs(t, f.tokens.Revoke(ctx, domain.Root(), "zz"), domain.ErrNotFound) | ||
| @@ -227,6 +228,56 @@ func TestTokensListRevoke(t *testing.T) { | |||||
| 227 | }) | 228 | }) | ||
| 228 | } | 229 | } | ||
| 229 | 230 | ||||
| 231 | + | // GO-003b: revoking a token with owner rights signs every browser out (the | |||
| 232 | + | // session epoch is bumped); revoking any other token does not. | |||
| 233 | + | func TestTokensRevokeOwnerRightsEndsWebSessions(t *testing.T) { | |||
| 234 | + | ctx := context.Background() | |||
| 235 | + | ownerTok := domain.Token{ID: "own", Scopes: domain.AllScopes()} | |||
| 236 | + | ||||
| 237 | + | for name, tc := range map[string]struct { | |||
| 238 | + | tok domain.Token | |||
| 239 | + | by domain.Principal | |||
| 240 | + | bumped bool | |||
| 241 | + | }{ | |||
| 242 | + | "every scope, by root": {ownerTok, domain.Root(), true}, | |||
| 243 | + | "every scope, by an owner key": {ownerTok, domain.TokenPrincipal(domain.Token{ID: "adm", Scopes: domain.AllScopes()}), true}, | |||
| 244 | + | "one scope short": {domain.Token{ID: "p", Scopes: domain.AllScopes()[1:]}, domain.Root(), false}, | |||
| 245 | + | "repo list": {domain.Token{ID: "l", Scopes: domain.AllScopes(), Repos: names(t, "a")}, domain.Root(), false}, | |||
| 246 | + | } { | |||
| 247 | + | t.Run(name, func(t *testing.T) { | |||
| 248 | + | f := newTokenFixture(t) | |||
| 249 | + | f.store.EXPECT().Get(ctx, tc.tok.ID).Return(tc.tok, nil) | |||
| 250 | + | f.store.EXPECT().Revoke(ctx, tc.tok.ID).Return(nil) | |||
| 251 | + | if tc.bumped { | |||
| 252 | + | f.sessions.EXPECT().BumpEpoch(ctx).Return(1, nil).Once() | |||
| 253 | + | } | |||
| 254 | + | require.NoError(t, f.tokens.Revoke(ctx, tc.by, tc.tok.ID)) | |||
| 255 | + | }) | |||
| 256 | + | } | |||
| 257 | + | ||||
| 258 | + | t.Run("a revoke that fails bumps nothing", func(t *testing.T) { | |||
| 259 | + | f := newTokenFixture(t) | |||
| 260 | + | f.store.EXPECT().Get(ctx, "own").Return(ownerTok, nil) | |||
| 261 | + | f.store.EXPECT().Revoke(ctx, "own").Return(errors.New("kv down")) | |||
| 262 | + | assert.Error(t, f.tokens.Revoke(ctx, domain.Root(), "own")) | |||
| 263 | + | }) | |||
| 264 | + | ||||
| 265 | + | t.Run("a token out of reach is not revoked and bumps nothing", func(t *testing.T) { | |||
| 266 | + | f := newTokenFixture(t) | |||
| 267 | + | limited := domain.TokenPrincipal(domain.Token{ID: "adm", Scopes: scopes(domain.ScopeTokenAdmin)}) | |||
| 268 | + | f.store.EXPECT().Get(ctx, "own").Return(ownerTok, nil) | |||
| 269 | + | assert.ErrorIs(t, f.tokens.Revoke(ctx, limited, "own"), domain.ErrNotFound) | |||
| 270 | + | }) | |||
| 271 | + | ||||
| 272 | + | t.Run("a failure to sign out is reported once the token is revoked", func(t *testing.T) { | |||
| 273 | + | f := newTokenFixture(t) | |||
| 274 | + | f.store.EXPECT().Get(ctx, "own").Return(ownerTok, nil) | |||
| 275 | + | f.store.EXPECT().Revoke(ctx, "own").Return(nil) | |||
| 276 | + | f.sessions.EXPECT().BumpEpoch(ctx).Return(0, errors.New("kv down")) | |||
| 277 | + | assert.ErrorContains(t, f.tokens.Revoke(ctx, domain.Root(), "own"), "signing out") | |||
| 278 | + | }) | |||
| 279 | + | } | |||
| 280 | + | ||||
| 230 | // A token admin sees and revokes only the tokens it could have created: the | 281 | // A token admin sees and revokes only the tokens it could have created: the | ||
| 231 | // no-escalation rule of Create, applied to scopes, repositories and expiry. | 282 | // no-escalation rule of Create, applied to scopes, repositories and expiry. | ||
| 232 | func TestTokensAdminReach(t *testing.T) { | 283 | func TestTokensAdminReach(t *testing.T) { | ||
| @@ -299,7 +350,9 @@ func TestTokensAdminReach(t *testing.T) { | |||||
| 299 | got, err := f.tokens.List(ctx, domain.Root()) | 350 | got, err := f.tokens.List(ctx, domain.Root()) | ||
| 300 | require.NoError(t, err) | 351 | require.NoError(t, err) | ||
| 301 | assert.Equal(t, all, got) | 352 | assert.Equal(t, all, got) | ||
| 302 | - | f.store.EXPECT().Revoke(ctx, "x").Return(nil) // no Get: root has no limits | 353 | + | f.store.EXPECT().Get(ctx, "x").Return(all[0], nil) |
| 354 | + | f.store.EXPECT().Revoke(ctx, "x").Return(nil) // root has no limits | |||
| 355 | + | f.sessions.EXPECT().BumpEpoch(ctx).Return(1, nil) // "x" has owner rights | |||
| 303 | require.NoError(t, f.tokens.Revoke(ctx, domain.Root(), "x")) | 356 | require.NoError(t, f.tokens.Revoke(ctx, domain.Root(), "x")) | ||
| 304 | }) | 357 | }) | ||
| 305 | 358 | ||||