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

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 1 addition & 2 deletions frontend/src/pages/authorize-page.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -184,8 +184,7 @@ export const AuthorizePage = () => {
<CardFooter className="flex flex-col items-stretch gap-3">
<Button
onClick={() => authorizeMutate()}
loading={authorizePending}
disabled={shouldAutoAuthorize}
loading={authorizePending || shouldAutoAuthorize}
>
{t("authorizeTitle")}
</Button>
Expand Down
3 changes: 1 addition & 2 deletions internal/bootstrap/app_bootstrap.go
Original file line number Diff line number Diff line change
Expand Up @@ -177,8 +177,7 @@ func (app *BootstrapApp) Setup() error {
cookieId := strings.Split(app.runtime.UUID, "-")[0] // first 8 characters of the uuid should be good enough

app.runtime.SessionCookieName = fmt.Sprintf("%s-%s", model.SessionCookieName, cookieId)
app.runtime.CSRFCookieName = fmt.Sprintf("%s-%s", model.CSRFCookieName, cookieId)
app.runtime.RedirectCookieName = fmt.Sprintf("%s-%s", model.RedirectCookieName, cookieId)
app.runtime.ScopeCookieName = fmt.Sprintf("%s-%s", model.OIDCScopeCookieName, cookieId)
app.runtime.OAuthSessionCookieName = fmt.Sprintf("%s-%s", model.OAuthSessionCookieName, cookieId)

// database
Expand Down
45 changes: 45 additions & 0 deletions internal/controller/oidc_controller.go
Original file line number Diff line number Diff line change
Expand Up @@ -34,6 +34,7 @@ type OIDCController struct {
log *logger.Logger
oidc *service.OIDCService
runtime *model.RuntimeConfig
config *model.Config
}

type AuthorizeCallback struct {
Expand Down Expand Up @@ -87,6 +88,7 @@ type OIDCControllerInput struct {

Log *logger.Logger
OIDCService *service.OIDCService
Config *model.Config
RuntimeConfig *model.RuntimeConfig
RouterGroup *gin.RouterGroup `name:"apiRouterGroup"`
MainRouter *gin.RouterGroup `name:"mainRouterGroup"`
Expand All @@ -97,6 +99,7 @@ func NewOIDCController(i OIDCControllerInput) *OIDCController {
log: i.Log,
oidc: i.OIDCService,
runtime: i.RuntimeConfig,
config: i.Config,
}

i.MainRouter.POST("/authorize", controller.authorize)
Expand Down Expand Up @@ -241,6 +244,19 @@ func (controller *OIDCController) authorize(c *gin.Context) {
}
}

cookieId := strings.SplitN(client.ClientID, "-", 2)[0]
cookieName := fmt.Sprintf("%s-%s", controller.runtime.ScopeCookieName, cookieId)
scopeCookie, err := c.Cookie(cookieName)

if err == nil {
scopes := fmt.Sprintf("scopes=%s;", req.Scope)
if controller.oidc.VerifySignedValue(client.ClientSecret, []byte(scopes), scopeCookie) {
if values.OIDCPrompt != service.OIDCPromptLogin {
values.OIDCPrompt = service.OIDCPromptNone
}
}
}

Comment on lines +247 to +259

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🔒 Security & Privacy | 🟠 Major | 🏗️ Heavy lift

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
set -euo pipefail

# Locate the OIDC controller and related consent-cookie code.
git ls-files | rg '^internal/controller/oidc_controller\.go$|^internal/.*\.go$' || true

echo "---- outline: internal/controller/oidc_controller.go ----"
ast-grep outline internal/controller/oidc_controller.go --view expanded || true

echo "---- relevant reads/writes ----"
rg -n "ScopeCookieName|VerifySignedValue|CreateSignedValue|OIDCPrompt|Authenticated|logout|Logout|scopeCookie|scopes=" internal/controller/oidc_controller.go internal -S

echo "---- surrounding lines around read site ----"
sed -n '230,280p' internal/controller/oidc_controller.go

echo "---- surrounding lines around write site ----"
sed -n '380,430p' internal/controller/oidc_controller.go

Repository: tinyauthapp/tinyauth

Length of output: 20696


🏁 Script executed:

#!/bin/bash
set -euo pipefail

echo "---- authorize() around prompt handling ----"
sed -n '150,260p' internal/controller/oidc_controller.go

echo "---- logout handler ----"
sed -n '230,310p' internal/controller/user_controller.go

echo "---- search for scope-cookie deletion/clearing ----"
rg -n "ScopeCookieName|SetCookie\\(|MaxAge: *-1|DeleteCookie|scope cookie|tinyauth-scope" internal -S

Repository: tinyauthapp/tinyauth

Length of output: 7322


🏁 Script executed:

#!/bin/bash
set -euo pipefail

echo "---- authorizeComplete() and nearby flow ----"
sed -n '280,380p' internal/controller/oidc_controller.go

echo "---- route wiring for OIDC endpoints ----"
rg -n "authorizeComplete|/oidc/authorize|authorize\\(" internal/bootstrap internal/controller internal -S

echo "---- OIDC controller tests mentioning prompt or consent cookies ----"
rg -n "prompt|scope cookie|consent|OIDCPromptNone|authorizeComplete|scopes=" internal/controller/oidc_controller_test.go internal/controller -S

Repository: tinyauthapp/tinyauth

Length of output: 8988


Bind consent to the authenticated user The scope cookie is browser-wide and not tied to the logged-in account, so a different user on the same browser can skip consent for the same client/scopes. It also survives logout because nothing clears it. Bind the signed value to the user identity (for example sub) and clear it on logout.

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@internal/controller/oidc_controller.go` around lines 247 - 259, Bind the
consent cookie to the authenticated user in the OIDC prompt handling around
VerifySignedValue: include the current user's stable identity, such as the `sub`
claim, in the signed payload and validate that same identity before skipping
consent. Update the consent-cookie creation logic to sign the identical
user-bound value, and clear or invalidate the cookie during logout so it cannot
survive account changes.

queries, err := query.Values(values)

if err != nil {
Expand Down Expand Up @@ -319,6 +335,19 @@ func (controller *OIDCController) authorizeComplete(c *gin.Context) {
return
}

// Get the client
client, ok := controller.oidc.GetClient(authorizeReq.ClientID)

if !ok {
controller.authorizeError(c, authorizeErrorParams{
err: errors.New("client not found"),
reason: "Client not found",
reasonPublic: "The client is not configured",
json: true,
})
return
}

// We no longer need the ticket
controller.oidc.DeleteAuthorizeRequestTicket(req.Ticket)

Expand Down Expand Up @@ -361,6 +390,22 @@ func (controller *OIDCController) authorizeComplete(c *gin.Context) {
return
}

// Set a cookie for the consent screen (approved scopes)
cookieId := strings.SplitN(client.ClientID, "-", 2)[0]
cookieName := fmt.Sprintf("%s-%s", controller.runtime.ScopeCookieName, cookieId)
scopes := fmt.Sprintf("scopes=%s;", authorizeReq.Scope)

cookie := &http.Cookie{
Name: cookieName,
Value: controller.oidc.CreateSignedValue(client.ClientSecret, []byte(scopes)),
Path: "/",
Secure: controller.config.Auth.SecureCookie,
HttpOnly: true,
SameSite: http.SameSiteLaxMode,
}

http.SetCookie(c.Writer, cookie)

c.JSON(200, gin.H{
"status": 200,
"redirect_uri": fmt.Sprintf("%s?%s", authorizeReq.RedirectURI, queries.Encode()),
Expand Down
3 changes: 1 addition & 2 deletions internal/model/constants.go
Original file line number Diff line number Diff line change
Expand Up @@ -20,8 +20,7 @@ var OverrideProviders = map[string]string{
var ReservedProviderNames = []string{"local", "ldap", "tailscale"}

const SessionCookieName = "tinyauth-session"
const CSRFCookieName = "tinyauth-csrf"
const RedirectCookieName = "tinyauth-redirect"
const OAuthSessionCookieName = "tinyauth-oauth"
const OIDCScopeCookieName = "tinyauth-scope"

const GracefulShutdownTimeout = 5 // seconds
3 changes: 1 addition & 2 deletions internal/model/runtime.go
Original file line number Diff line number Diff line change
Expand Up @@ -5,8 +5,7 @@ type RuntimeConfig struct {
UUID string
CookieDomain string
SessionCookieName string
CSRFCookieName string
RedirectCookieName string
ScopeCookieName string
OAuthSessionCookieName string
LocalUsers []LocalUser
OAuthProviders map[string]OAuthServiceConfig
Expand Down
26 changes: 26 additions & 0 deletions internal/service/oidc_service.go
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@ package service
import (
"context"
"crypto"
"crypto/hmac"
"crypto/rand"
"crypto/rsa"
"crypto/sha256"
Expand All @@ -22,6 +23,7 @@ import (

"github.com/go-jose/go-jose/v4"
"github.com/golang-jwt/jwt/v5"
"github.com/google/uuid"
"github.com/steveiliop56/ding"
"github.com/tinyauthapp/tinyauth/internal/model"
"github.com/tinyauthapp/tinyauth/internal/repository"
Expand Down Expand Up @@ -305,6 +307,9 @@ func NewOIDCService(i OIDCServiceInput) (*OIDCService, error) {

for id, client := range i.Config.OIDC.Clients {
client.ID = id
if err := uuid.Validate(client.ClientID); err != nil {
return nil, fmt.Errorf("invalid client id: %w", err)
}
if client.Name == "" {
client.Name = utils.Capitalize(client.ID)
}
Expand All @@ -318,6 +323,9 @@ func NewOIDCService(i OIDCServiceInput) (*OIDCService, error) {
client.ClientSecret = secret
}
client.ClientSecretFile = ""
if len(client.ClientSecret) < 32 {
return nil, fmt.Errorf("client secret for client %s is too short, must be >= 32 chars", client.ClientID)
}
clients[id] = client
i.Log.App.Debug().Str("clientId", client.ClientID).Msg("Loaded OIDC client configuration")
}
Expand Down Expand Up @@ -969,3 +977,21 @@ func (service *OIDCService) GetPrompt(prompt string) []OIDCPrompt {

return parsedPromps
}

func (service *OIDCService) CreateSignedValue(key string, data []byte) string {
// create the signature
h := hmac.New(sha256.New, []byte(key))
h.Write(data)
sig := base64.URLEncoding.EncodeToString(h.Sum(nil))

// hash the data
hasher := sha256.New()
hasher.Write(data)
hash := base64.URLEncoding.EncodeToString(hasher.Sum(nil))

return fmt.Sprintf("%s.%s", hash, sig)
}

func (service *OIDCService) VerifySignedValue(key string, data []byte, signedValue string) bool {
return service.CreateSignedValue(key, data) == signedValue
}
Comment on lines +995 to +997

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🔒 Security & Privacy | 🟠 Major | ⚡ Quick win

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
set -euo pipefail

# Inspect the relevant file and surrounding context.
sed -n '1,120p' internal/service/oidc_service.go
printf '\n----\n'
sed -n '940,1020p' internal/service/oidc_service.go
printf '\n----\n'

# Find related symbols/usages.
rg -n "VerifySignedValue|CreateSignedValue|hmac\.Equal|Verify.*Signed" internal/service/oidc_service.go internal -S

Repository: tinyauthapp/tinyauth

Length of output: 7359


Use constant-time comparison for the signature check. CreateSignedValue(...) == signedValue compares the HMAC with a regular string comparison and leaks timing differences; use hmac.Equal here instead.

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@internal/service/oidc_service.go` around lines 995 - 997, Replace the regular
string comparison in OIDCService.VerifySignedValue with hmac.Equal, comparing
the generated signature and signedValue as byte slices to ensure constant-time
verification.

Loading