diff --git a/pkg/models/api_routes.go b/pkg/models/api_routes.go index e236d79eb..ecd8441ee 100644 --- a/pkg/models/api_routes.go +++ b/pkg/models/api_routes.go @@ -251,6 +251,7 @@ func CollectRoutesForAPITokenUsage(route echo.RouteInfo, requiresJWT bool) { routeGroupName == "subscriptions" || routeGroupName == "tokens" || routeGroupName == "*" || + routeGroupName == "oauth_authorize" || strings.HasPrefix(routeGroupName, "user_") { return } diff --git a/pkg/modules/auth/oauth2server/authorize.go b/pkg/modules/auth/oauth2server/authorize.go index 96afbbad7..0cad63083 100644 --- a/pkg/modules/auth/oauth2server/authorize.go +++ b/pkg/modules/auth/oauth2server/authorize.go @@ -47,6 +47,10 @@ type AuthorizeResponse struct { // It validates the OAuth parameters, creates an authorization code, and // returns it as JSON. Authentication is handled by the token middleware. func HandleAuthorize(c *echo.Context) error { + if c.Get("api_token") != nil { + return echo.NewHTTPError(http.StatusForbidden, "API tokens cannot be used to authorize OAuth clients") + } + var req AuthorizeRequest if err := c.Bind(&req); err != nil { return echo.NewHTTPError(http.StatusBadRequest, "Invalid request body") diff --git a/pkg/routes/api/v2/oauth.go b/pkg/routes/api/v2/oauth.go index a67441ad3..7343cc4e3 100644 --- a/pkg/routes/api/v2/oauth.go +++ b/pkg/routes/api/v2/oauth.go @@ -82,6 +82,10 @@ func oauthToken(ctx context.Context, in *struct { } func oauthAuthorize(ctx context.Context, in *struct{ Body oauth2server.AuthorizeRequest }) (*oauthAuthorizeBody, error) { + if ec := echoContextFromCtx(ctx); ec != nil && (*ec).Get("api_token") != nil { + return nil, huma.Error403Forbidden("API tokens cannot be used to authorize OAuth clients") + } + a, err := authFromCtx(ctx) if err != nil { return nil, err diff --git a/pkg/webtests/huma_auth_test.go b/pkg/webtests/huma_auth_test.go index 404f89f00..442ca7f09 100644 --- a/pkg/webtests/huma_auth_test.go +++ b/pkg/webtests/huma_auth_test.go @@ -212,6 +212,17 @@ func TestHumaOAuth(t *testing.T) { assert.Equal(t, http.StatusUnauthorized, rec.Code) }) + // GHSA-v3p6-34mc-hj7v: an oauth-scoped API token must not obtain an + // authorization code (and with it a full session JWT). + t.Run("authorize rejects legacy oauth-scoped API token", func(t *testing.T) { + apiToken := insertLegacyOAuthScopedToken(t) + + body := authorizeRequestBody("code", "vikunja", "vikunja-flutter://callback", "abc", "S256", "") + rec := humaRequest(t, e, http.MethodPost, "/api/v2/oauth/authorize", string(body), apiToken, "") + assert.Equal(t, http.StatusUnauthorized, rec.Code) + assert.NotContains(t, rec.Body.String(), `"code":"`) + }) + t.Run("full code flow with PKCE (JSON token request)", func(t *testing.T) { verifier := "dBjftJeZ4CVP-mB92K27uhbUJU1p1r_wW1gFWFOEjXk" challenge := pkceChallenge(verifier) diff --git a/pkg/webtests/oauth2_test.go b/pkg/webtests/oauth2_test.go index b1ec97eb3..0acd07a23 100644 --- a/pkg/webtests/oauth2_test.go +++ b/pkg/webtests/oauth2_test.go @@ -24,10 +24,14 @@ import ( "net/http" "net/http/httptest" "testing" + "time" + "code.vikunja.io/api/pkg/db" + "code.vikunja.io/api/pkg/models" "code.vikunja.io/api/pkg/modules/auth" "code.vikunja.io/api/pkg/modules/auth/oauth2server" + "github.com/labstack/echo/v5" "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" ) @@ -55,6 +59,34 @@ func doAuthorize(e http.Handler, token string, body []byte) *httptest.ResponseRe return rec } +// insertLegacyOAuthScopedToken inserts an API token scoped {"oauth":["authorize"]} +// directly into the db, bypassing Create's permission validation, to simulate a +// token issued before the oauth scope was removed from the grantable catalogue +// (GHSA-v3p6-34mc-hj7v). Returns the cleartext token. +func insertLegacyOAuthScopedToken(t *testing.T) string { + t.Helper() + + const cleartext = "tk_legacy_oauth_scoped_token_0000deadbeef" + const salt = "legacysalt" + token := &models.APIToken{ + Title: "legacy oauth token", + TokenSalt: salt, + TokenHash: models.HashToken(cleartext, salt), + TokenLastEight: cleartext[len(cleartext)-8:], + APIPermissions: models.APIPermissions{"oauth": {"authorize"}}, + ExpiresAt: time.Now().Add(24 * time.Hour), + OwnerID: 1, + } + + s := db.NewSession() + defer s.Close() + _, err := s.Insert(token) + require.NoError(t, err) + require.NoError(t, s.Commit()) + + return cleartext +} + // doTokenRequest performs a JSON POST to /api/v1/oauth/token and returns the recorder. func doTokenRequest(e http.Handler, params map[string]string) *httptest.ResponseRecorder { body, _ := json.Marshal(params) //nolint:errchkjson @@ -121,6 +153,44 @@ func TestOAuth2AuthorizeEndpoint(t *testing.T) { rec := doAuthorize(e, token, body) assert.Equal(t, http.StatusBadRequest, rec.Code) }) + + // GHSA-v3p6-34mc-hj7v: a token scoped {"oauth":["authorize"]} could mint a + // full session JWT via the authorize + token endpoints. + t.Run("rejects legacy oauth-scoped API token", func(t *testing.T) { + e, err := setupTestEnv() + require.NoError(t, err) + + apiToken := insertLegacyOAuthScopedToken(t) + + body := authorizeRequestBody("code", "vikunja", "vikunja-flutter://callback", "abc123", "S256", "") + rec := doAuthorize(e, apiToken, body) + assert.Equal(t, http.StatusUnauthorized, rec.Code) + assert.NotContains(t, rec.Body.String(), `"code":"`) + }) + + t.Run("rejects API-token principal at the handler", func(t *testing.T) { + e, err := setupTestEnv() + require.NoError(t, err) + + body := authorizeRequestBody("code", "vikunja", "vikunja-flutter://callback", "abc123", "S256", "") + req := httptest.NewRequest(http.MethodPost, "/api/v1/oauth/authorize", bytes.NewReader(body)) + req.Header.Set("Content-Type", "application/json") + c := e.NewContext(req, httptest.NewRecorder()) + c.Set("api_token", &models.APIToken{ID: 1}) + + err = oauth2server.HandleAuthorize(c) + var httpErr *echo.HTTPError + require.ErrorAs(t, err, &httpErr) + assert.Equal(t, http.StatusForbidden, httpErr.Code) + }) + + t.Run("oauth scope is not grantable", func(t *testing.T) { + _, err := setupTestEnv() + require.NoError(t, err) + + require.Error(t, models.PermissionsAreValid(models.APIPermissions{"oauth": {"authorize"}})) + assert.NotContains(t, models.GetAPITokenRoutes(), "oauth") + }) } // getAuthorizationCode performs the authorize step and returns the code from the JSON response.