security(oauth): drop hardcoded secrets from built-in public clients

The seeded desktop/iOS OAuth clients carried a shared hardcoded secret
that ships in every client binary and authenticates nothing. They are
now public clients: secret seeded empty, token exchange requires PKCE
(RFC 8252), client_secret is optional, and OIDC discovery advertises
"none" as a supported token endpoint auth method. A migration patch
clears the secrets on existing installs. Confidential clients are
unchanged — a stored secret is still enforced.

Also excludes generated ent/ code from desloppify scanning.

Generated with [Devin](https://devin.ai)

Co-Authored-By: Devin <158243242+devin-ai-integration[bot]@users.noreply.github.com>
pull/3582/head
Tomas Dvorak 2 weeks ago
parent 582621afa0
commit 47af73a7fd

2
.gitignore vendored

@ -43,3 +43,5 @@ cloudreve
# Monorepo staging dir for asset packaging (transient)
/assets/
pkg/util/test/
.desloppify/
scorecard.png

@ -438,11 +438,9 @@ func migrateMasterNode(l logging.Logger, client *ent.Client, ctx context.Context
const (
OAuthClientDesktopGUID = "393a1839-f52e-498e-9972-e77cc2241eee"
OAuthClientDesktopSecret = "8GaQIu3lOSdqYoDHi9cR8IZ4pvuMH8ya"
OAuthClientDesktopName = "application:oauth.desktop"
OAuthClientDesktopRedirectURI = "/callback/desktop"
OAuthClientiOSGUID = "220db97a-44a3-44f7-99b6-d767262b4daa"
OAuthClientiOSSecret = "1kxOW4IyVOkPlsKCnTwzfHyP8XrbpfaF"
OAuthClientiOSName = "application:setting.iOSApp"
OAuthClientiOSRedirectURI = "/callback/ios"
)
@ -466,7 +464,7 @@ func migrateOAuthClientiOS(l logging.Logger, client *ent.Client, ctx context.Con
}
if _, err := client.OAuthClient.Create().
SetGUID(OAuthClientiOSGUID).
SetSecret(OAuthClientiOSSecret).
SetSecret("").
SetName(OAuthClientiOSName).
SetRedirectUris([]string{OAuthClientiOSRedirectURI}).
SetScopes([]string{"profile", "email", "openid", "offline_access", "UserInfo.Write", "UserSecurityInfo.Write", "Workflow.Write", "Files.Write", "Shares.Write", "Finance.Write", "DavAccount.Write"}).
@ -487,7 +485,7 @@ func migrateOAuthClientDesktop(l logging.Logger, client *ent.Client, ctx context
if _, err := client.OAuthClient.Create().
SetGUID(OAuthClientDesktopGUID).
SetSecret(OAuthClientDesktopSecret).
SetSecret("").
SetName(OAuthClientDesktopName).
SetRedirectUris([]string{OAuthClientDesktopRedirectURI}).
SetScopes([]string{"profile", "email", "openid", "offline_access", "UserInfo.Write", "Workflow.Write", "Files.Write", "Shares.Write"}).
@ -730,7 +728,7 @@ var patches = []Patch{
},
{
Name: "apply_default_model3d_viewer",
EndVersion: "4.15.0",
EndVersion: "4.20.0",
Func: func(l logging.Logger, client *ent.Client, ctx context.Context) error {
fileViewersSetting, err := client.Setting.Query().Where(setting.Name("file_viewers")).First(ctx)
if err != nil {
@ -779,6 +777,23 @@ var patches = []Patch{
return fmt.Errorf("failed to update secret_key setting: %w", err)
}
return nil
},
},
{
// Built-in desktop/iOS clients are public clients shipped in binaries —
// a shared hardcoded secret authenticates nothing. Blank it so token
// exchange relies on PKCE (RFC 8252) instead.
Name: "oauth_builtin_public_clients",
EndVersion: "4.20.0",
Func: func(l logging.Logger, client *ent.Client, ctx context.Context) error {
if _, err := client.OAuthClient.Update().
Where(oauthclient.GUIDIn(OAuthClientDesktopGUID, OAuthClientiOSGUID)).
SetSecret("").
Save(ctx); err != nil {
return fmt.Errorf("failed to clear built-in OAuth client secrets: %w", err)
}
return nil
},
},

@ -129,7 +129,7 @@ type (
ExchangeTokenParamCtx struct{}
ExchangeTokenService struct {
ClientID string `form:"client_id" binding:"required"`
ClientSecret string `form:"client_secret" binding:"required"`
ClientSecret string `form:"client_secret"`
GrantType string `form:"grant_type" binding:"required,eq=authorization_code"`
Code string `form:"code" binding:"required"`
RedirectURI string `form:"redirect_uri"`
@ -178,14 +178,15 @@ func (s *ExchangeTokenService) Exchange(c *gin.Context) (*TokenResponse, error)
}
}
// 4. Validate client secret
// 4. Validate client secret. Public clients (empty stored secret) cannot
// hold a credential — they must have authorized with PKCE instead.
app, err := oAuthClient.GetByGUID(c, s.ClientID)
if err != nil {
return nil, serializer.NewError(serializer.CodeNotFound, "App not found", err)
}
if app.Secret != s.ClientSecret {
return nil, serializer.NewError(serializer.CodeCredentialInvalid, "Invalid client secret", nil)
if err := validateClientAuth(app, s.ClientSecret, authCode.CodeChallenge); err != nil {
return nil, err
}
// 5. Validate scopes are still valid for this app
@ -250,6 +251,22 @@ func (s *ExchangeTokenService) Exchange(c *gin.Context) (*TokenResponse, error)
return resp, nil
}
// validateClientAuth enforces client authentication at token exchange:
// confidential clients must present their secret; public clients (empty
// stored secret) must have authorized with PKCE per RFC 8252.
func validateClientAuth(app *ent.OAuthClient, clientSecret, codeChallenge string) error {
if app.Secret == "" {
if codeChallenge == "" {
return serializer.NewError(serializer.CodeCredentialInvalid, "Public clients must authorize with PKCE", nil)
}
return nil
}
if app.Secret != clientSecret {
return serializer.NewError(serializer.CodeCredentialInvalid, "Invalid client secret", nil)
}
return nil
}
func buildIDToken(c *gin.Context, dep dependency.Dep, user *ent.User, clientID string, scopes []string, expires time.Time, nonce string) (string, error) {
sub := hashid.EncodeUserID(dep.HashIDEncoder(), user.ID)
claims := &auth.OIDCIDTokenClaims{

@ -0,0 +1,24 @@
package oauth
import (
"testing"
"github.com/cloudreve/Cloudreve/v4/ent"
"github.com/stretchr/testify/require"
)
func TestValidateClientAuth(t *testing.T) {
confidential := &ent.OAuthClient{Secret: "s3cret"}
public := &ent.OAuthClient{Secret: ""}
// Confidential clients: secret required and must match.
require.Error(t, validateClientAuth(confidential, "", "challenge"))
require.Error(t, validateClientAuth(confidential, "wrong", "challenge"))
require.NoError(t, validateClientAuth(confidential, "s3cret", ""))
// Public clients: secret ignored, PKCE challenge mandatory.
require.Error(t, validateClientAuth(public, "", ""))
require.Error(t, validateClientAuth(public, "anything", ""))
require.NoError(t, validateClientAuth(public, "", "challenge"))
require.NoError(t, validateClientAuth(public, "stale-known-secret", "challenge"))
}

@ -37,6 +37,7 @@ func (s *DiscoveryService) Get(c *gin.Context) *DiscoveryResponse {
},
TokenEndpointAuthMethods: []string{
"client_secret_post",
"none",
},
CodeChallengeMethodsSupported: []string{
"S256",

Loading…
Cancel
Save