From 4be667bceafc61b638be34bdd56fc6cdfdbe6889 Mon Sep 17 00:00:00 2001 From: Mitchell Macpherson Date: Thu, 29 Feb 2024 13:19:45 +1100 Subject: [PATCH] fix: Role checking & improved JWT Signing with an ephemeral key instead of user-provided information --- .gitignore | 1 + module_app.go | 25 ++++++++++++++++++------- module_callback.go | 25 +++++++++++++++++++------ module_entrypoint.go | 23 ++++++++++------------- utils.go | 9 +++++++++ 5 files changed, 57 insertions(+), 26 deletions(-) diff --git a/.gitignore b/.gitignore index b708f9f..59e0cfe 100644 --- a/.gitignore +++ b/.gitignore @@ -1,3 +1,4 @@ /.idea/ /Caddyfile vendor/ +/dist/ diff --git a/module_app.go b/module_app.go index cc5737d..9445633 100644 --- a/module_app.go +++ b/module_app.go @@ -1,6 +1,7 @@ package caddydiscord import ( + "encoding/hex" "fmt" "github.com/caddyserver/caddy/v2" "github.com/caddyserver/caddy/v2/caddyconfig/httpcaddyfile" @@ -53,8 +54,20 @@ type DiscordPortalApp struct { // Realms group together explicit rules about whom to authorise. Realms RealmRegistry `json:"realms"` oauthConfig *oauth2.Config - Key string `json:"key,omitempty"` - Signature string `json:"signature,omitempty"` + + // Key is the signing key used for the JWT stored as the client's cookie + // it is ephemeral alongside the caddy server process. + Key string `json:"key,omitempty"` + + // ExecutionKey is an ephemeral identifier for the client's cookie which contains + // the JWT payload proving Discord identity. It is the 'public' version of the signing Key. + // + // End users will have to perform the OAuth flow once uniquely per ExecutionKey, + // which will be a touchless experience barely noticeably from their end. + // + // ExecutionKey exists as an alternative to the server operator providing their own + // JWT signing key; which should eventually become an optional configuration. + ExecutionKey string `json:"signature,omitempty"` } // CaddyModule returns the Caddy module information. @@ -67,11 +80,9 @@ func (DiscordPortalApp) CaddyModule() caddy.ModuleInfo { func (d *DiscordPortalApp) Provision(_ caddy.Context) error { // Discord App ID is used as entropy for JWT signing keys. - d.Key = hashString512(d.ClientID) + d.Key = hex.EncodeToString(randomness(64)) + d.ExecutionKey = hashString512(d.Key) - // TODO: Signature will be used for cookie integrity checks, to ensure checks are inline with most recent Caddyfile. - // TODOTODO: Use parsed caddyfile signature for checks, instead of just Discord App Client ID. - d.Signature = hashString256(d.ClientID, 16) return nil } @@ -94,7 +105,7 @@ func (d DiscordPortalApp) Validate() error { } if d.RedirectURL == "" { - return fmt.Errorf("redirect URL has not bee configured") + return fmt.Errorf("redirect URL is not configured") } return nil diff --git a/module_callback.go b/module_callback.go index 7240434..cdeaa95 100644 --- a/module_callback.go +++ b/module_callback.go @@ -3,7 +3,6 @@ package caddydiscord import ( "context" "encoding/hex" - "fmt" "github.com/caddyserver/caddy/v2" "github.com/caddyserver/caddy/v2/caddyconfig/caddyfile" "github.com/caddyserver/caddy/v2/caddyconfig/httpcaddyfile" @@ -45,7 +44,7 @@ type DiscordAuthPlugin struct { Key string tokenSigner TokenSignerSignature flowTokenParser FlowTokenParserSignature - signature string + cookie CookieNamer } func (DiscordAuthPlugin) CaddyModule() caddy.ModuleInfo { @@ -60,6 +59,7 @@ func (s *DiscordAuthPlugin) Provision(ctx caddy.Context) error { app := ctxApp.(*DiscordPortalApp) s.OAuth = app.getOAuthConfig() + s.cookie = CookieName(app.ExecutionKey) s.Realms = &app.Realms key, err := hex.DecodeString(app.Key) @@ -69,7 +69,6 @@ func (s *DiscordAuthPlugin) Provision(ctx caddy.Context) error { s.tokenSigner = NewTokenSigner(key) s.flowTokenParser = NewFlowTokenParser(key) - s.signature = app.Signature return nil } @@ -147,14 +146,28 @@ func (d DiscordAuthPlugin) ServeHTTP(w http.ResponseWriter, r *http.Request, _ c if rule.Resource == DiscordRoleRule { matchedRole := RoleChecker(rule.Identifier, guildMembership.Roles) - // Found a valid role assigned. if matchedRole != "" { + // Authorised based on role whitelist. allowed = true break } } - allowed = true + if rule.Resource == DiscordGuildRule { + if rule.Wildcard == true { + // Authorised based on wildcard user within guild. + allowed = true + break + } + } + + if rule.Resource == DiscordMemberRule { + if identity.ID == rule.Identifier { + // Authorised based on user whitelist. + allowed = true + break + } + } } else if rule.Resource == DiscordUserRule && rule.Wildcard == false && rule.Identifier == identity.ID { allowed = true break @@ -182,7 +195,7 @@ func (d DiscordAuthPlugin) ServeHTTP(w http.ResponseWriter, r *http.Request, _ c } cookie := &http.Cookie{ - Name: fmt.Sprintf("%s_%s", cookieName, realm.Ref), + Name: d.cookie(realm.Ref), Value: signedToken, Expires: expiration, HttpOnly: true, diff --git a/module_entrypoint.go b/module_entrypoint.go index bb7dd10..0cdd562 100644 --- a/module_entrypoint.go +++ b/module_entrypoint.go @@ -2,7 +2,6 @@ package caddydiscord import ( "encoding/hex" - "fmt" "github.com/caddyserver/caddy/v2" "github.com/caddyserver/caddy/v2/caddyconfig" "github.com/caddyserver/caddy/v2/caddyconfig/caddyfile" @@ -36,11 +35,6 @@ func parseCaddyfileHandlerDirective2(h httpcaddyfile.Helper) (caddyhttp.Middlewa } -type ProtectCfg struct { - ClientID string - ClientSecret string -} - // ProtectorPlugin allows you to authenticate caddy routes from // a Discord User Identity. // @@ -57,13 +51,15 @@ type ProtectorPlugin struct { authedTokenParser AuthedTokenParserSignature flowTokenParser FlowTokenParserSignature Realm string + cookie CookieNamer } // Authenticate implements caddyhttp.MiddlewareHandler. -func (e ProtectorPlugin) Authenticate(w http.ResponseWriter, r *http.Request) (caddyauth.User, bool, error) { - existingSession, _ := r.Cookie(fmt.Sprintf("%s_%s", cookieName, e.Realm)) +func (p *ProtectorPlugin) Authenticate(w http.ResponseWriter, r *http.Request) (caddyauth.User, bool, error) { + existingSession, _ := r.Cookie(p.cookie(p.Realm)) // Handle passing through signed token over to support multiple domains. + // TODO: Refactor this code into oblivion. if existingSession == nil && r.URL.Query().Has("DISCO_PASSTHROUGH") && r.URL.Query().Has("DISCO_REALM") { q := r.URL.Query() signedToken := q.Get("DISCO_PASSTHROUGH") @@ -75,7 +71,7 @@ func (e ProtectorPlugin) Authenticate(w http.ResponseWriter, r *http.Request) (c // TODO: Expires should be reduced if authorisation failed. cookie := &http.Cookie{ - Name: fmt.Sprintf("%s_%s", cookieName, realm), + Name: p.cookie(realm), Value: signedToken, Expires: time.Now().Add(time.Hour * 16), HttpOnly: true, @@ -90,7 +86,7 @@ func (e ProtectorPlugin) Authenticate(w http.ResponseWriter, r *http.Request) (c } if existingSession != nil { - claims, err := e.authedTokenParser(existingSession.Value) + claims, err := p.authedTokenParser(existingSession.Value) if err != nil { return caddyauth.User{}, false, err } @@ -115,15 +111,15 @@ func (e ProtectorPlugin) Authenticate(w http.ResponseWriter, r *http.Request) (c backToURL.Host = r.Host } - token := NewAuthFlowToken(backToURL.String(), e.Realm, exp) - signedToken, err := e.tokenSigner(token) + token := NewAuthFlowToken(backToURL.String(), p.Realm, exp) + signedToken, err := p.tokenSigner(token) if err != nil { // Unable to generate JWT http.Error(w, "Failed to generate token", http.StatusInternalServerError) return caddyauth.User{}, false, err } - url := e.OAuthConfig.AuthCodeURL(signedToken, oauth2.SetAuthURLParam("prompt", "none")) + url := p.OAuthConfig.AuthCodeURL(signedToken, oauth2.SetAuthURLParam("prompt", "none")) http.Redirect(w, r, url, http.StatusTemporaryRedirect) return caddyauth.User{}, false, nil @@ -132,6 +128,7 @@ func (e ProtectorPlugin) Authenticate(w http.ResponseWriter, r *http.Request) (c func (p *ProtectorPlugin) Provision(ctx caddy.Context) error { ctxApp, _ := ctx.App(moduleName) app := ctxApp.(*DiscordPortalApp) + p.cookie = CookieName(app.ExecutionKey) p.OAuthConfig = app.getOAuthConfig() key, err := hex.DecodeString(app.Key) diff --git a/utils.go b/utils.go index 128f5e7..b6d2390 100644 --- a/utils.go +++ b/utils.go @@ -2,6 +2,7 @@ package caddydiscord import ( "crypto/rand" + "fmt" ) func randomness(length uint) []byte { @@ -12,3 +13,11 @@ func randomness(length uint) []byte { } return randomBytes } + +type CookieNamer func(string) string + +func CookieName(executionKey string) CookieNamer { + return func(realm string) string { + return fmt.Sprintf("%s_%s_%s", cookieName, realm, executionKey) + } +}