mirror of
https://github.com/go-vikunja/vikunja.git
synced 2026-07-22 12:06:11 -05:00
fix(auth): reject API tokens at the OAuth authorize endpoint (GHSA-v3p6-34mc-hj7v)
This commit is contained in:
@@ -251,6 +251,7 @@ func CollectRoutesForAPITokenUsage(route echo.RouteInfo, requiresJWT bool) {
|
||||
routeGroupName == "subscriptions" ||
|
||||
routeGroupName == "tokens" ||
|
||||
routeGroupName == "*" ||
|
||||
routeGroupName == "oauth_authorize" ||
|
||||
strings.HasPrefix(routeGroupName, "user_") {
|
||||
return
|
||||
}
|
||||
|
||||
@@ -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")
|
||||
|
||||
@@ -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
|
||||
|
||||
@@ -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)
|
||||
|
||||
@@ -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.
|
||||
|
||||
Reference in New Issue
Block a user