OAuth public client support through DCR and PKCE support for public/confidential clients (#33664)

* public client support along with PKCE for public/confidential clients

---------

Co-authored-by: Mattermost Build <build@mattermost.com>
This commit is contained in:
Ben Cooke 2025-11-11 12:43:37 -05:00 committed by GitHub
parent a9c9953439
commit a79ac96b50
No known key found for this signature in database
GPG key ID: B5690EEEBB952194
22 changed files with 1579 additions and 209 deletions

View file

@ -48,6 +48,7 @@ func createOAuthApp(c *Context, w http.ResponseWriter, r *http.Request) {
}
oauthApp.CreatorId = c.AppContext.Session().UserId
oauthApp.IsDynamicallyRegistered = false
rapp, err := c.App.CreateOAuthApp(&oauthApp)
if err != nil {

View file

@ -786,3 +786,39 @@ func TestRegisterOAuthClient_DisabledFeatures(t *testing.T) {
require.Error(t, err)
CheckBadRequestStatus(t, resp)
}
func TestRegisterOAuthClient_PublicClient_Success(t *testing.T) {
// Test successful public client DCR registration
mainHelper.Parallel(t)
th := Setup(t)
defer th.TearDown()
client := th.Client
th.App.UpdateConfig(func(cfg *model.Config) {
*cfg.ServiceSettings.EnableOAuthServiceProvider = true
cfg.ServiceSettings.EnableDynamicClientRegistration = model.NewPointer(true)
})
// DCR request for public client
request := &model.ClientRegistrationRequest{
RedirectURIs: []string{"https://example.com/callback"},
TokenEndpointAuthMethod: model.NewPointer(model.ClientAuthMethodNone),
ClientName: model.NewPointer("Test Public Client"),
ClientURI: model.NewPointer("https://example.com"),
}
// Register public client
response, resp, err := client.RegisterOAuthClient(context.Background(), request)
require.NoError(t, err)
CheckCreatedStatus(t, resp)
require.NotNil(t, response)
// Verify response properties for public client
assert.NotEmpty(t, response.ClientID)
assert.Nil(t, response.ClientSecret) // No client secret for public clients
assert.Equal(t, request.RedirectURIs, response.RedirectURIs)
assert.Equal(t, model.ClientAuthMethodNone, response.TokenEndpointAuthMethod)
assert.Equal(t, *request.ClientName, *response.ClientName)
assert.Equal(t, *request.ClientURI, *response.ClientURI)
assert.Equal(t, "user", response.Scope)
}

View file

@ -6,7 +6,6 @@ package app
import (
"bytes"
"context"
"crypto/subtle"
b64 "encoding/base64"
"encoding/json"
"fmt"
@ -168,7 +167,16 @@ func (a *App) GetOAuthImplicitRedirect(rctx request.CTX, userID string, authRequ
}
func (a *App) GetOAuthCodeRedirect(userID string, authRequest *model.AuthorizeRequest) (string, *model.AppError) {
authData := &model.AuthData{UserId: userID, ClientId: authRequest.ClientId, CreateAt: model.GetMillis(), RedirectUri: authRequest.RedirectURI, State: authRequest.State, Scope: authRequest.Scope}
authData := &model.AuthData{
UserId: userID,
ClientId: authRequest.ClientId,
CreateAt: model.GetMillis(),
RedirectUri: authRequest.RedirectURI,
State: authRequest.State,
Scope: authRequest.Scope,
CodeChallenge: authRequest.CodeChallenge,
CodeChallengeMethod: authRequest.CodeChallengeMethod,
}
authData.Code = model.NewId() + model.NewId()
// parse authRequest.RedirectURI to handle query parameters see: https://mattermost.atlassian.net/browse/MM-46216
@ -213,6 +221,11 @@ func (a *App) AllowOAuthAppAccessToUser(rctx request.CTX, userID string, authReq
return "", model.NewAppError("AllowOAuthAppAccessToUser", "api.oauth.allow_oauth.redirect_callback.app_error", nil, "", http.StatusBadRequest)
}
// Validate PKCE requirements for public clients
if oauthApp.IsPublicClient() && authRequest.ResponseType == model.AuthCodeResponseType && authRequest.CodeChallenge == "" {
return "", model.NewAppError("AllowOAuthAppAccessToUser", "api.oauth.allow_oauth.pkce_required_public.app_error", nil, "", http.StatusBadRequest)
}
var redirectURI string
var err *model.AppError
switch authRequest.ResponseType {
@ -274,7 +287,7 @@ func (a *App) GetOAuthAccessTokenForImplicitFlow(rctx request.CTX, userID string
return session, nil
}
func (a *App) GetOAuthAccessTokenForCodeFlow(rctx request.CTX, clientId, grantType, redirectURI, code, secret, refreshToken string) (*model.AccessResponse, *model.AppError) {
func (a *App) GetOAuthAccessTokenForCodeFlow(rctx request.CTX, clientId, grantType, redirectURI, code, secret, refreshToken, codeVerifier string) (*model.AccessResponse, *model.AppError) {
if !*a.Config().ServiceSettings.EnableOAuthServiceProvider {
return nil, model.NewAppError("GetOAuthAccessToken", "api.oauth.get_access_token.disabled.app_error", nil, "", http.StatusNotImplemented)
}
@ -284,107 +297,150 @@ func (a *App) GetOAuthAccessTokenForCodeFlow(rctx request.CTX, clientId, grantTy
return nil, model.NewAppError("GetOAuthAccessToken", "api.oauth.get_access_token.credentials.app_error", nil, "", http.StatusNotFound).Wrap(nErr)
}
if subtle.ConstantTimeCompare([]byte(oauthApp.ClientSecret), []byte(secret)) == 0 {
return nil, model.NewAppError("GetOAuthAccessToken", "api.oauth.get_access_token.credentials.app_error", nil, "", http.StatusForbidden)
if err := a.validateOAuthClient(oauthApp, grantType, secret, codeVerifier); err != nil {
return nil, err
}
var accessData *model.AccessData
var accessRsp *model.AccessResponse
var user *model.User
if grantType == model.AccessTokenGrantType {
var authData *model.AuthData
authData, nErr = a.Srv().Store().OAuth().GetAuthData(code)
if nErr != nil {
return nil, model.NewAppError("GetOAuthAccessToken", "api.oauth.get_access_token.expired_code.app_error", nil, "", http.StatusBadRequest).Wrap(nErr)
}
return a.handleAuthorizationCodeGrant(rctx, oauthApp, redirectURI, code, codeVerifier, clientId)
}
if authData.IsExpired() {
if nErr = a.Srv().Store().OAuth().RemoveAuthData(authData.Code); nErr != nil {
rctx.Logger().Warn("unable to remove auth data", mlog.Err(nErr))
}
return nil, model.NewAppError("GetOAuthAccessToken", "api.oauth.get_access_token.expired_code.app_error", nil, "", http.StatusForbidden)
}
return a.handleRefreshTokenGrant(rctx, oauthApp, refreshToken)
}
if authData.RedirectUri != redirectURI {
return nil, model.NewAppError("GetOAuthAccessToken", "api.oauth.get_access_token.redirect_uri.app_error", nil, "", http.StatusBadRequest)
}
func (a *App) validateOAuthClient(oauthApp *model.OAuthApp, grantType, secret, codeVerifier string) *model.AppError {
return oauthApp.ValidateForGrantType(grantType, secret, codeVerifier)
}
user, nErr = a.Srv().Store().User().Get(context.Background(), authData.UserId)
if nErr != nil {
return nil, model.NewAppError("GetOAuthAccessToken", "api.oauth.get_access_token.internal_user.app_error", nil, "", http.StatusNotFound).Wrap(nErr)
}
func (a *App) validatePKCE(oauthApp *model.OAuthApp, authData *model.AuthData, codeVerifier string) *model.AppError {
return authData.ValidatePKCEForClientType(oauthApp.IsPublicClient(), codeVerifier)
}
if user.DeleteAt != 0 {
return nil, model.NewAppError("GetOAuthAccessToken", "api.oauth.get_access_token.expired_code.app_error", nil, "", http.StatusForbidden)
}
accessData, nErr = a.Srv().Store().OAuth().GetPreviousAccessData(user.Id, clientId)
if nErr != nil {
return nil, model.NewAppError("GetOAuthAccessToken", "api.oauth.get_access_token.internal.app_error", nil, "", http.StatusBadRequest).Wrap(nErr)
}
if accessData != nil {
if accessData.IsExpired() {
var access *model.AccessResponse
access, err := a.newSessionUpdateToken(rctx, oauthApp, accessData, user)
if err != nil {
return nil, err
}
accessRsp = access
} else {
// Return the same token and no need to create a new session
accessRsp = &model.AccessResponse{
AccessToken: accessData.Token,
TokenType: model.AccessTokenType,
RefreshToken: accessData.RefreshToken,
ExpiresInSeconds: int32((accessData.ExpiresAt - model.GetMillis()) / 1000),
}
}
} else {
var session *model.Session
// Create a new session and return new access token
session, err := a.newSession(rctx, oauthApp, user)
if err != nil {
return nil, err
}
accessData = &model.AccessData{ClientId: clientId, UserId: user.Id, Token: session.Token, RefreshToken: model.NewId(), RedirectUri: redirectURI, ExpiresAt: session.ExpiresAt, Scope: authData.Scope}
if _, nErr = a.Srv().Store().OAuth().SaveAccessData(accessData); nErr != nil {
return nil, model.NewAppError("GetOAuthAccessToken", "api.oauth.get_access_token.internal_saving.app_error", nil, "", http.StatusInternalServerError).Wrap(nErr)
}
accessRsp = &model.AccessResponse{
AccessToken: session.Token,
TokenType: model.AccessTokenType,
RefreshToken: accessData.RefreshToken,
ExpiresInSeconds: int32(*a.Config().ServiceSettings.SessionLengthSSOInHours * 60 * 60),
}
}
func (a *App) handleAuthorizationCodeGrant(rctx request.CTX, oauthApp *model.OAuthApp, redirectURI, code, codeVerifier, clientId string) (*model.AccessResponse, *model.AppError) {
authData, nErr := a.Srv().Store().OAuth().GetAuthData(code)
if nErr != nil {
return nil, model.NewAppError("GetOAuthAccessToken", "api.oauth.get_access_token.expired_code.app_error", nil, "", http.StatusBadRequest).Wrap(nErr)
}
if authData.IsExpired() {
if nErr = a.Srv().Store().OAuth().RemoveAuthData(authData.Code); nErr != nil {
rctx.Logger().Warn("unable to remove auth data", mlog.Err(nErr))
}
} else {
// When grantType is refresh_token
accessData, nErr = a.Srv().Store().OAuth().GetAccessDataByRefreshToken(refreshToken)
if nErr != nil {
return nil, model.NewAppError("GetOAuthAccessToken", "api.oauth.get_access_token.refresh_token.app_error", nil, "", http.StatusNotFound).Wrap(nErr)
}
user, nErr := a.Srv().Store().User().Get(context.Background(), accessData.UserId)
if nErr != nil {
return nil, model.NewAppError("GetOAuthAccessToken", "api.oauth.get_access_token.internal_user.app_error", nil, "", http.StatusNotFound).Wrap(nErr)
}
access, err := a.newSessionUpdateToken(rctx, oauthApp, accessData, user)
if err != nil {
return nil, err
}
accessRsp = access
return nil, model.NewAppError("GetOAuthAccessToken", "api.oauth.get_access_token.expired_code.app_error", nil, "", http.StatusForbidden)
}
return accessRsp, nil
if authData.RedirectUri != redirectURI {
return nil, model.NewAppError("GetOAuthAccessToken", "api.oauth.get_access_token.redirect_uri.app_error", nil, "", http.StatusBadRequest)
}
if err := a.validatePKCE(oauthApp, authData, codeVerifier); err != nil {
return nil, err
}
user, nErr := a.Srv().Store().User().Get(context.Background(), authData.UserId)
if nErr != nil {
return nil, model.NewAppError("GetOAuthAccessToken", "api.oauth.get_access_token.internal_user.app_error", nil, "", http.StatusNotFound).Wrap(nErr)
}
if user.DeleteAt != 0 {
return nil, model.NewAppError("GetOAuthAccessToken", "api.oauth.get_access_token.expired_code.app_error", nil, "", http.StatusForbidden)
}
defer func() {
if nErr = a.Srv().Store().OAuth().RemoveAuthData(authData.Code); nErr != nil {
rctx.Logger().Warn("unable to remove auth data", mlog.Err(nErr))
}
}()
return a.generateAccessTokenResponse(rctx, oauthApp, user, clientId, redirectURI, authData.Scope)
}
func (a *App) handleRefreshTokenGrant(rctx request.CTX, oauthApp *model.OAuthApp, refreshToken string) (*model.AccessResponse, *model.AppError) {
// Validate that this client can use refresh token grant type
if err := oauthApp.ValidateForGrantType(model.RefreshTokenGrantType, oauthApp.ClientSecret, ""); err != nil {
return nil, err
}
accessData, nErr := a.Srv().Store().OAuth().GetAccessDataByRefreshToken(refreshToken)
if nErr != nil {
return nil, model.NewAppError("GetOAuthAccessToken", "api.oauth.get_access_token.refresh_token.app_error", nil, "", http.StatusNotFound).Wrap(nErr)
}
user, nErr := a.Srv().Store().User().Get(context.Background(), accessData.UserId)
if nErr != nil {
return nil, model.NewAppError("GetOAuthAccessToken", "api.oauth.get_access_token.internal_user.app_error", nil, "", http.StatusNotFound).Wrap(nErr)
}
return a.newSessionUpdateToken(rctx, oauthApp, accessData, user)
}
func (a *App) generateAccessTokenResponse(rctx request.CTX, oauthApp *model.OAuthApp, user *model.User, clientId, redirectURI, scope string) (*model.AccessResponse, *model.AppError) {
accessData, nErr := a.Srv().Store().OAuth().GetPreviousAccessData(user.Id, clientId)
if nErr != nil {
return nil, model.NewAppError("GetOAuthAccessToken", "api.oauth.get_access_token.internal.app_error", nil, "", http.StatusBadRequest).Wrap(nErr)
}
if accessData != nil {
return a.handleExistingAccessData(rctx, oauthApp, accessData, user)
}
return a.createNewAccessData(rctx, oauthApp, user, clientId, redirectURI, scope)
}
func (a *App) handleExistingAccessData(rctx request.CTX, oauthApp *model.OAuthApp, accessData *model.AccessData, user *model.User) (*model.AccessResponse, *model.AppError) {
if accessData.IsExpired() {
return a.newSessionUpdateToken(rctx, oauthApp, accessData, user)
}
refreshToken := accessData.RefreshToken
if oauthApp.IsPublicClient() {
refreshToken = ""
}
return &model.AccessResponse{
AccessToken: accessData.Token,
TokenType: model.AccessTokenType,
RefreshToken: refreshToken,
ExpiresInSeconds: int32((accessData.ExpiresAt - model.GetMillis()) / 1000),
}, nil
}
func (a *App) createNewAccessData(rctx request.CTX, oauthApp *model.OAuthApp, user *model.User, clientId, redirectURI, scope string) (*model.AccessResponse, *model.AppError) {
session, err := a.newSession(rctx, oauthApp, user)
if err != nil {
return nil, err
}
refreshToken := ""
if !oauthApp.IsPublicClient() {
refreshToken = model.NewId()
}
accessData := &model.AccessData{
ClientId: clientId,
UserId: user.Id,
Token: session.Token,
RefreshToken: refreshToken,
RedirectUri: redirectURI,
ExpiresAt: session.ExpiresAt,
Scope: scope,
}
if _, nErr := a.Srv().Store().OAuth().SaveAccessData(accessData); nErr != nil {
return nil, model.NewAppError("GetOAuthAccessToken", "api.oauth.get_access_token.internal_saving.app_error", nil, "", http.StatusInternalServerError).Wrap(nErr)
}
refreshTokenResponse := accessData.RefreshToken
if oauthApp.IsPublicClient() {
refreshTokenResponse = ""
}
return &model.AccessResponse{
AccessToken: session.Token,
TokenType: model.AccessTokenType,
RefreshToken: refreshTokenResponse,
ExpiresInSeconds: int32(*a.Config().ServiceSettings.SessionLengthSSOInHours * 60 * 60),
}, nil
}
func (a *App) newSession(rctx request.CTX, app *model.OAuthApp, user *model.User) (*model.Session, *model.AppError) {
@ -427,7 +483,12 @@ func (a *App) newSessionUpdateToken(rctx request.CTX, app *model.OAuthApp, acces
}
accessData.Token = session.Token
accessData.RefreshToken = model.NewId()
// Generate refresh token only for confidential clients
if !app.IsPublicClient() {
accessData.RefreshToken = model.NewId()
} else {
accessData.RefreshToken = ""
}
accessData.ExpiresAt = session.ExpiresAt
if _, err := a.Srv().Store().OAuth().UpdateAccessData(accessData); err != nil {
@ -1071,9 +1132,30 @@ func (a *App) GetAuthorizationServerMetadata(rctx request.CTX) (*model.Authoriza
}
func (a *App) RegisterOAuthClient(rctx request.CTX, req *model.ClientRegistrationRequest, userID string) (*model.OAuthApp, *model.AppError) {
if !*a.Config().ServiceSettings.EnableOAuthServiceProvider {
return nil, model.NewAppError("RegisterOAuthClient", "api.oauth.register_oauth_app.turn_off.app_error", nil, "", http.StatusNotImplemented)
}
app := model.NewOAuthAppFromClientRegistration(req, userID)
return a.CreateOAuthApp(app)
oauthApp, err := a.Srv().Store().OAuth().SaveApp(app)
if err != nil {
var appErr *model.AppError
var invErr *store.ErrInvalidInput
a.Log().Error("Error saving OAuth app via DCR", mlog.Err(err), mlog.String("name", app.Name))
switch {
case errors.As(err, &appErr):
return nil, appErr
case errors.As(err, &invErr):
return nil, model.NewAppError("RegisterOAuthClient", "app.oauth.save_app.existing.app_error", nil, "", http.StatusBadRequest).Wrap(err)
default:
return nil, model.NewAppError("RegisterOAuthClient", "app.oauth.save_app.save.app_error", nil, "", http.StatusInternalServerError).Wrap(err)
}
}
return oauthApp, nil
}
// parseOAuthStateTokenExtra parses a token extra string in the format "email:action:cookie".

View file

@ -29,49 +29,212 @@ func TestGetOAuthAccessTokenForImplicitFlow(t *testing.T) {
th := Setup(t).InitBasic()
defer th.TearDown()
th.App.UpdateConfig(func(cfg *model.Config) { *cfg.ServiceSettings.EnableOAuthServiceProvider = true })
t.Run("BasicFlow_Success", func(t *testing.T) {
th.App.UpdateConfig(func(cfg *model.Config) { *cfg.ServiceSettings.EnableOAuthServiceProvider = true })
oapp := &model.OAuthApp{
Name: "fakeoauthapp" + model.NewRandomString(10),
CreatorId: th.BasicUser2.Id,
Homepage: "https://nowhere.com",
Description: "test",
CallbackUrls: []string{"https://nowhere.com"},
}
oapp := &model.OAuthApp{
Name: "fakeoauthapp" + model.NewRandomString(10),
CreatorId: th.BasicUser2.Id,
Homepage: "https://nowhere.com",
Description: "test",
CallbackUrls: []string{"https://nowhere.com"},
}
oapp, err := th.App.CreateOAuthApp(oapp)
require.Nil(t, err)
oapp, err := th.App.CreateOAuthApp(oapp)
require.Nil(t, err)
authRequest := &model.AuthorizeRequest{
ResponseType: model.ImplicitResponseType,
ClientId: oapp.Id,
RedirectURI: oapp.CallbackUrls[0],
Scope: "",
State: "123",
}
authRequest := &model.AuthorizeRequest{
ResponseType: model.ImplicitResponseType,
ClientId: oapp.Id,
RedirectURI: oapp.CallbackUrls[0],
Scope: "",
State: "123",
}
session, err := th.App.GetOAuthAccessTokenForImplicitFlow(th.Context, th.BasicUser.Id, authRequest)
assert.Nil(t, err)
assert.NotNil(t, session)
session, err := th.App.GetOAuthAccessTokenForImplicitFlow(th.Context, th.BasicUser.Id, authRequest)
assert.Nil(t, err)
assert.NotNil(t, session)
})
th.App.UpdateConfig(func(cfg *model.Config) { *cfg.ServiceSettings.EnableOAuthServiceProvider = false })
t.Run("OAuthDisabled_ShouldFail", func(t *testing.T) {
th.App.UpdateConfig(func(cfg *model.Config) { *cfg.ServiceSettings.EnableOAuthServiceProvider = true })
session, err = th.App.GetOAuthAccessTokenForImplicitFlow(th.Context, th.BasicUser.Id, authRequest)
assert.NotNil(t, err, "should fail - oauth2 disabled")
assert.Nil(t, session)
oapp := &model.OAuthApp{
Name: "fakeoauthapp" + model.NewRandomString(10),
CreatorId: th.BasicUser2.Id,
Homepage: "https://nowhere.com",
Description: "test",
CallbackUrls: []string{"https://nowhere.com"},
}
th.App.UpdateConfig(func(cfg *model.Config) { *cfg.ServiceSettings.EnableOAuthServiceProvider = true })
authRequest.ClientId = "junk"
oapp, err := th.App.CreateOAuthApp(oapp)
require.Nil(t, err)
session, err = th.App.GetOAuthAccessTokenForImplicitFlow(th.Context, th.BasicUser.Id, authRequest)
assert.NotNil(t, err, "should fail - bad client id")
assert.Nil(t, session)
th.App.UpdateConfig(func(cfg *model.Config) { *cfg.ServiceSettings.EnableOAuthServiceProvider = false })
authRequest.ClientId = oapp.Id
authRequest := &model.AuthorizeRequest{
ResponseType: model.ImplicitResponseType,
ClientId: oapp.Id,
RedirectURI: oapp.CallbackUrls[0],
Scope: "",
State: "123",
}
session, err = th.App.GetOAuthAccessTokenForImplicitFlow(th.Context, "junk", authRequest)
assert.NotNil(t, err, "should fail - bad user id")
assert.Nil(t, session)
session, err := th.App.GetOAuthAccessTokenForImplicitFlow(th.Context, th.BasicUser.Id, authRequest)
assert.NotNil(t, err)
assert.Nil(t, session)
})
t.Run("BadClientId_ShouldFail", func(t *testing.T) {
th.App.UpdateConfig(func(cfg *model.Config) { *cfg.ServiceSettings.EnableOAuthServiceProvider = true })
authRequest := &model.AuthorizeRequest{
ResponseType: model.ImplicitResponseType,
ClientId: "invalid_client_id",
RedirectURI: "https://nowhere.com",
Scope: "",
State: "123",
}
session, err := th.App.GetOAuthAccessTokenForImplicitFlow(th.Context, th.BasicUser.Id, authRequest)
assert.NotNil(t, err)
assert.Nil(t, session)
})
t.Run("BadUserId_ShouldFail", func(t *testing.T) {
th.App.UpdateConfig(func(cfg *model.Config) { *cfg.ServiceSettings.EnableOAuthServiceProvider = true })
oapp := &model.OAuthApp{
Name: "fakeoauthapp" + model.NewRandomString(10),
CreatorId: th.BasicUser2.Id,
Homepage: "https://nowhere.com",
Description: "test",
CallbackUrls: []string{"https://nowhere.com"},
}
oapp, err := th.App.CreateOAuthApp(oapp)
require.Nil(t, err)
authRequest := &model.AuthorizeRequest{
ResponseType: model.ImplicitResponseType,
ClientId: oapp.Id,
RedirectURI: oapp.CallbackUrls[0],
Scope: "",
State: "123",
}
session, err := th.App.GetOAuthAccessTokenForImplicitFlow(th.Context, "invalid_user_id", authRequest)
assert.NotNil(t, err)
assert.Nil(t, session)
})
t.Run("PublicClient_Success", func(t *testing.T) {
th.App.UpdateConfig(func(cfg *model.Config) { *cfg.ServiceSettings.EnableOAuthServiceProvider = true })
dcrRequest := &model.ClientRegistrationRequest{
ClientName: model.NewPointer("Public Client Test"),
RedirectURIs: []string{"https://example.com/callback"},
TokenEndpointAuthMethod: model.NewPointer(model.ClientAuthMethodNone),
ClientURI: model.NewPointer("https://example.com"),
}
publicApp, appErr := th.App.RegisterOAuthClient(th.Context, dcrRequest, th.BasicUser2.Id)
require.Nil(t, appErr)
require.Empty(t, publicApp.ClientSecret)
authRequest := &model.AuthorizeRequest{
ResponseType: model.ImplicitResponseType,
ClientId: publicApp.Id,
RedirectURI: publicApp.CallbackUrls[0],
Scope: "user",
State: "test_state",
}
redirectURL, appErr := th.App.AllowOAuthAppAccessToUser(th.Context, th.BasicUser.Id, authRequest)
require.Nil(t, appErr)
require.Contains(t, redirectURL, "#access_token=")
require.Contains(t, redirectURL, "token_type=bearer")
require.Contains(t, redirectURL, "state=test_state")
// Parse the access token from the fragment
uri, err := url.Parse(redirectURL)
require.NoError(t, err)
fragment := uri.Fragment
fragmentValues, err := url.ParseQuery(fragment)
require.NoError(t, err)
accessToken := fragmentValues.Get("access_token")
require.NotEmpty(t, accessToken)
// Verify session exists
session, appErr := th.App.GetSession(accessToken)
require.Nil(t, appErr)
require.NotNil(t, session)
require.Equal(t, th.BasicUser.Id, session.UserId)
require.True(t, session.IsOAuth)
// Verify access data exists for public client
accessData, err := th.App.Srv().Store().OAuth().GetAccessData(accessToken)
require.NoError(t, err)
require.NotNil(t, accessData)
require.Equal(t, publicApp.Id, accessData.ClientId)
require.Equal(t, th.BasicUser.Id, accessData.UserId)
require.Empty(t, accessData.RefreshToken)
})
t.Run("ConfidentialClient_Success", func(t *testing.T) {
th.App.UpdateConfig(func(cfg *model.Config) { *cfg.ServiceSettings.EnableOAuthServiceProvider = true })
confidentialApp := &model.OAuthApp{
Name: "Confidential Client Test",
CreatorId: th.BasicUser2.Id,
Homepage: "https://example.com",
Description: "test confidential client",
CallbackUrls: []string{"https://example.com/callback"},
ClientSecret: model.NewId(),
}
confidentialApp, appErr := th.App.CreateOAuthApp(confidentialApp)
require.Nil(t, appErr)
require.NotEmpty(t, confidentialApp.ClientSecret)
authRequest := &model.AuthorizeRequest{
ResponseType: model.ImplicitResponseType,
ClientId: confidentialApp.Id,
RedirectURI: confidentialApp.CallbackUrls[0],
Scope: "user",
State: "test_state",
}
redirectURL, appErr := th.App.AllowOAuthAppAccessToUser(th.Context, th.BasicUser.Id, authRequest)
require.Nil(t, appErr)
require.Contains(t, redirectURL, "#access_token=")
require.Contains(t, redirectURL, "token_type=bearer")
require.Contains(t, redirectURL, "state=test_state")
// Parse the access token from the fragment
uri, err := url.Parse(redirectURL)
require.NoError(t, err)
fragment := uri.Fragment
fragmentValues, err := url.ParseQuery(fragment)
require.NoError(t, err)
accessToken := fragmentValues.Get("access_token")
require.NotEmpty(t, accessToken)
// Verify session exists
session, appErr := th.App.GetSession(accessToken)
require.Nil(t, appErr)
require.NotNil(t, session)
require.Equal(t, th.BasicUser.Id, session.UserId)
require.True(t, session.IsOAuth)
// Verify access data exists for confidential client
accessData, err := th.App.Srv().Store().OAuth().GetAccessData(accessToken)
require.NoError(t, err)
require.NotNil(t, accessData)
require.Equal(t, confidentialApp.Id, accessData.ClientId)
require.Equal(t, th.BasicUser.Id, accessData.UserId)
require.Empty(t, accessData.RefreshToken)
})
}
func TestOAuthRevokeAccessToken(t *testing.T) {
@ -690,7 +853,7 @@ func TestDeactivatedUserOAuthApp(t *testing.T) {
_, appErr = th.App.UpdateActive(th.Context, th.BasicUser, false)
require.Nil(t, appErr)
resp, appErr := th.App.GetOAuthAccessTokenForCodeFlow(th.Context, oapp.Id, model.AccessTokenGrantType, oapp.CallbackUrls[0], code, oapp.ClientSecret, "")
resp, appErr := th.App.GetOAuthAccessTokenForCodeFlow(th.Context, oapp.Id, model.AccessTokenGrantType, oapp.CallbackUrls[0], code, oapp.ClientSecret, "", "")
assert.Nil(t, resp)
require.NotNil(t, appErr, "Should not get access token")
require.Equal(t, http.StatusBadRequest, appErr.StatusCode)
@ -755,6 +918,27 @@ func TestRegisterOAuthClient(t *testing.T) {
require.NotNil(t, appErr)
assert.Equal(t, "model.oauth.is_valid.homepage.app_error", appErr.Id)
})
t.Run("PublicClient_Success", func(t *testing.T) {
th.App.UpdateConfig(func(cfg *model.Config) {
cfg.ServiceSettings.EnableDynamicClientRegistration = model.NewPointer(true)
})
dcrRequest := &model.ClientRegistrationRequest{
RedirectURIs: []string{"https://example.com/callback"},
ClientName: model.NewPointer("Test Public Client"),
TokenEndpointAuthMethod: model.NewPointer(model.ClientAuthMethodNone),
}
registeredApp, appErr := th.App.RegisterOAuthClient(th.Context, dcrRequest, "")
require.Nil(t, appErr)
require.NotNil(t, registeredApp)
require.Empty(t, registeredApp.ClientSecret)
require.True(t, registeredApp.IsPublicClient())
require.Equal(t, model.ClientAuthMethodNone, registeredApp.GetTokenEndpointAuthMethod())
require.True(t, registeredApp.IsDynamicallyRegistered)
})
}
func TestGetAuthorizationServerMetadata_DCRConfig(t *testing.T) {
@ -804,6 +988,265 @@ func TestGetAuthorizationServerMetadata_DCRConfig(t *testing.T) {
})
}
func TestGetOAuthAccessTokenForCodeFlow(t *testing.T) {
mainHelper.Parallel(t)
th := Setup(t).InitBasic()
defer th.TearDown()
th.App.UpdateConfig(func(cfg *model.Config) { *cfg.ServiceSettings.EnableOAuthServiceProvider = true })
t.Run("PublicClient_WithPKCE_Success", func(t *testing.T) {
dcrRequest := &model.ClientRegistrationRequest{
ClientName: model.NewPointer("Public Client Test"),
RedirectURIs: []string{"https://example.com/callback"},
TokenEndpointAuthMethod: model.NewPointer(model.ClientAuthMethodNone),
ClientURI: model.NewPointer("https://example.com"),
}
publicApp, appErr := th.App.RegisterOAuthClient(th.Context, dcrRequest, th.BasicUser2.Id)
require.Nil(t, appErr)
require.Empty(t, publicApp.ClientSecret)
codeVerifier := "dBjftJeZ4CVP-mB92K27uhbUJU1p1r_wW1gFWFOEjXk"
codeChallenge := "E9Melhoa2OwvFrEMTJguCHaoeK1t8URWbuGJSstw-cM"
codeChallengeMethod := model.PKCECodeChallengeMethodS256
authRequest := &model.AuthorizeRequest{
ResponseType: model.ResponseTypeCode,
ClientId: publicApp.Id,
RedirectURI: publicApp.CallbackUrls[0],
Scope: "user",
State: "test_state",
CodeChallenge: codeChallenge,
CodeChallengeMethod: codeChallengeMethod,
}
redirectURL, appErr := th.App.AllowOAuthAppAccessToUser(th.Context, th.BasicUser.Id, authRequest)
require.Nil(t, appErr)
uri, err := url.Parse(redirectURL)
require.NoError(t, err)
code := uri.Query().Get("code")
require.NotEmpty(t, code)
accessResponse, appErr := th.App.GetOAuthAccessTokenForCodeFlow(
th.Context,
publicApp.Id,
model.AccessTokenGrantType,
authRequest.RedirectURI,
code,
"",
"",
codeVerifier,
)
require.Nil(t, appErr)
require.NotNil(t, accessResponse)
require.NotEmpty(t, accessResponse.AccessToken)
require.Equal(t, model.AccessTokenType, accessResponse.TokenType)
require.Empty(t, accessResponse.RefreshToken)
})
t.Run("PublicClient_WithoutPKCE_ShouldFail", func(t *testing.T) {
dcrRequest := &model.ClientRegistrationRequest{
ClientName: model.NewPointer("Public Client Test"),
RedirectURIs: []string{"https://example.com/callback"},
TokenEndpointAuthMethod: model.NewPointer(model.ClientAuthMethodNone),
ClientURI: model.NewPointer("https://example.com"),
}
publicApp, appErr := th.App.RegisterOAuthClient(th.Context, dcrRequest, th.BasicUser2.Id)
require.Nil(t, appErr)
authRequest := &model.AuthorizeRequest{
ResponseType: model.ResponseTypeCode,
ClientId: publicApp.Id,
RedirectURI: publicApp.CallbackUrls[0],
Scope: "user",
State: "test_state",
}
_, appErr = th.App.AllowOAuthAppAccessToUser(th.Context, th.BasicUser.Id, authRequest)
require.NotNil(t, appErr)
require.Contains(t, appErr.Id, "pkce_required")
})
t.Run("ConfidentialClient_WithPKCE_Success", func(t *testing.T) {
confidentialApp := &model.OAuthApp{
Name: "Confidential Client Test",
CreatorId: th.BasicUser2.Id,
Homepage: "https://example.com",
Description: "test confidential client",
CallbackUrls: []string{"https://example.com/callback"},
ClientSecret: model.NewId(),
}
confidentialApp, appErr := th.App.CreateOAuthApp(confidentialApp)
require.Nil(t, appErr)
require.NotEmpty(t, confidentialApp.ClientSecret)
codeVerifier := "dBjftJeZ4CVP-mB92K27uhbUJU1p1r_wW1gFWFOEjXk"
codeChallenge := "E9Melhoa2OwvFrEMTJguCHaoeK1t8URWbuGJSstw-cM"
codeChallengeMethod := model.PKCECodeChallengeMethodS256
authRequest := &model.AuthorizeRequest{
ResponseType: model.ResponseTypeCode,
ClientId: confidentialApp.Id,
RedirectURI: confidentialApp.CallbackUrls[0],
Scope: "user",
State: "test_state",
CodeChallenge: codeChallenge,
CodeChallengeMethod: codeChallengeMethod,
}
redirectURL, appErr := th.App.AllowOAuthAppAccessToUser(th.Context, th.BasicUser.Id, authRequest)
require.Nil(t, appErr)
uri, err := url.Parse(redirectURL)
require.NoError(t, err)
code := uri.Query().Get("code")
require.NotEmpty(t, code)
accessResponse, appErr := th.App.GetOAuthAccessTokenForCodeFlow(
th.Context,
confidentialApp.Id,
model.AccessTokenGrantType,
authRequest.RedirectURI,
code,
confidentialApp.ClientSecret,
"",
codeVerifier,
)
require.Nil(t, appErr)
require.NotNil(t, accessResponse)
require.NotEmpty(t, accessResponse.AccessToken)
require.Equal(t, model.AccessTokenType, accessResponse.TokenType)
require.NotEmpty(t, accessResponse.RefreshToken)
})
t.Run("ConfidentialClient_WithoutPKCE_Success", func(t *testing.T) {
confidentialApp := &model.OAuthApp{
Name: "Confidential Client Test",
CreatorId: th.BasicUser2.Id,
Homepage: "https://example.com",
Description: "test confidential client",
CallbackUrls: []string{"https://example.com/callback"},
ClientSecret: model.NewId(),
}
confidentialApp, appErr := th.App.CreateOAuthApp(confidentialApp)
require.Nil(t, appErr)
authRequest := &model.AuthorizeRequest{
ResponseType: model.ResponseTypeCode,
ClientId: confidentialApp.Id,
RedirectURI: confidentialApp.CallbackUrls[0],
Scope: "user",
State: "test_state",
}
redirectURL, appErr := th.App.AllowOAuthAppAccessToUser(th.Context, th.BasicUser.Id, authRequest)
require.Nil(t, appErr)
uri, err := url.Parse(redirectURL)
require.NoError(t, err)
code := uri.Query().Get("code")
require.NotEmpty(t, code)
accessResponse, appErr := th.App.GetOAuthAccessTokenForCodeFlow(
th.Context,
confidentialApp.Id,
model.AccessTokenGrantType,
authRequest.RedirectURI,
code,
confidentialApp.ClientSecret,
"",
"",
)
require.Nil(t, appErr)
require.NotNil(t, accessResponse)
require.NotEmpty(t, accessResponse.AccessToken)
require.Equal(t, model.AccessTokenType, accessResponse.TokenType)
require.NotEmpty(t, accessResponse.RefreshToken)
})
t.Run("ConfidentialClient_PKCEEnforcement", func(t *testing.T) {
confidentialApp := &model.OAuthApp{
Name: "Confidential Client Test",
CreatorId: th.BasicUser2.Id,
Homepage: "https://example.com",
Description: "test confidential client",
CallbackUrls: []string{"https://example.com/callback"},
ClientSecret: model.NewId(),
}
confidentialApp, appErr := th.App.CreateOAuthApp(confidentialApp)
require.Nil(t, appErr)
codeChallenge := "E9Melhoa2OwvFrEMTJguCHaoeK1t8URWbuGJSstw-cM"
codeChallengeMethod := model.PKCECodeChallengeMethodS256
authRequest := &model.AuthorizeRequest{
ResponseType: model.ResponseTypeCode,
ClientId: confidentialApp.Id,
RedirectURI: confidentialApp.CallbackUrls[0],
Scope: "user",
State: "test_state",
CodeChallenge: codeChallenge,
CodeChallengeMethod: codeChallengeMethod,
}
redirectURL, appErr := th.App.AllowOAuthAppAccessToUser(th.Context, th.BasicUser.Id, authRequest)
require.Nil(t, appErr)
uri, err := url.Parse(redirectURL)
require.NoError(t, err)
code := uri.Query().Get("code")
require.NotEmpty(t, code)
_, appErr = th.App.GetOAuthAccessTokenForCodeFlow(
th.Context,
confidentialApp.Id,
model.AccessTokenGrantType,
authRequest.RedirectURI,
code,
confidentialApp.ClientSecret,
"",
"",
)
require.NotNil(t, appErr)
require.Contains(t, appErr.Id, "pkce")
})
t.Run("PublicClient_NoRefreshToken", func(t *testing.T) {
dcrRequest := &model.ClientRegistrationRequest{
ClientName: model.NewPointer("Public Client Test"),
RedirectURIs: []string{"https://example.com/callback"},
TokenEndpointAuthMethod: model.NewPointer(model.ClientAuthMethodNone),
ClientURI: model.NewPointer("https://example.com"),
}
publicApp, appErr := th.App.RegisterOAuthClient(th.Context, dcrRequest, th.BasicUser2.Id)
require.Nil(t, appErr)
_, appErr = th.App.GetOAuthAccessTokenForCodeFlow(
th.Context,
publicApp.Id,
model.RefreshTokenGrantType,
"https://example.com/callback",
"",
"",
"some_fake_refresh_token",
"",
)
require.NotNil(t, appErr)
require.Contains(t, appErr.Id, "public_client_refresh_token.app_error")
})
}
func TestParseOAuthStateTokenExtra(t *testing.T) {
t.Run("valid token with normal values", func(t *testing.T) {
email, action, cookie, err := parseOAuthStateTokenExtra("user@example.com:email_to_sso:randomcookie123")

View file

@ -285,3 +285,5 @@ channels/db/migrations/postgres/000143_content_flagging_table_index.down.sql
channels/db/migrations/postgres/000143_content_flagging_table_index.up.sql
channels/db/migrations/postgres/000144_add_dcr_fields_to_oauth_apps.down.sql
channels/db/migrations/postgres/000144_add_dcr_fields_to_oauth_apps.up.sql
channels/db/migrations/postgres/000145_add_pkce_to_oauthauthdata.down.sql
channels/db/migrations/postgres/000145_add_pkce_to_oauthauthdata.up.sql

View file

@ -0,0 +1,2 @@
ALTER TABLE oauthauthdata DROP COLUMN IF EXISTS codechallenge;
ALTER TABLE oauthauthdata DROP COLUMN IF EXISTS codechallengemethod;

View file

@ -0,0 +1,2 @@
ALTER TABLE oauthauthdata ADD COLUMN IF NOT EXISTS codechallenge varchar(128) DEFAULT '';
ALTER TABLE oauthauthdata ADD COLUMN IF NOT EXISTS codechallengemethod varchar(10) DEFAULT '';

View file

@ -36,7 +36,7 @@ func newSqlOAuthStore(sqlStore *SqlStore) store.OAuthStore {
From("OAuthAccessData")
s.oAuthAuthDataQuery = s.getQueryBuilder().
Select("ClientId", "UserId", "Code", "ExpiresIn", "CreateAt", "RedirectUri", "State", "Scope").
Select("ClientId", "UserId", "Code", "ExpiresIn", "CreateAt", "RedirectUri", "State", "Scope", "CodeChallenge", "CodeChallengeMethod").
From("OAuthAuthData")
return &s
@ -273,9 +273,9 @@ func (as SqlOAuthStore) SaveAuthData(authData *model.AuthData) (*model.AuthData,
}
if _, err := as.GetMaster().NamedExec(`INSERT INTO OAuthAuthData
(ClientId, UserId, Code, ExpiresIn, CreateAt, RedirectUri, State, Scope)
(ClientId, UserId, Code, ExpiresIn, CreateAt, RedirectUri, State, Scope, CodeChallenge, CodeChallengeMethod)
VALUES
(:ClientId, :UserId, :Code, :ExpiresIn, :CreateAt, :RedirectUri, :State, :Scope)`, authData); err != nil {
(:ClientId, :UserId, :Code, :ExpiresIn, :CreateAt, :RedirectUri, :State, :Scope, :CodeChallenge, :CodeChallengeMethod)`, authData); err != nil {
return nil, errors.Wrap(err, "failed to save AuthData")
}
return authData, nil

View file

@ -125,11 +125,13 @@ func authorizeOAuthPage(c *Context, w http.ResponseWriter, r *http.Request) {
}
authRequest := &model.AuthorizeRequest{
ResponseType: r.URL.Query().Get("response_type"),
ClientId: r.URL.Query().Get("client_id"),
RedirectURI: r.URL.Query().Get("redirect_uri"),
Scope: r.URL.Query().Get("scope"),
State: r.URL.Query().Get("state"),
ResponseType: r.URL.Query().Get("response_type"),
ClientId: r.URL.Query().Get("client_id"),
RedirectURI: r.URL.Query().Get("redirect_uri"),
Scope: r.URL.Query().Get("scope"),
State: r.URL.Query().Get("state"),
CodeChallenge: r.URL.Query().Get("code_challenge"),
CodeChallengeMethod: r.URL.Query().Get("code_challenge_method"),
}
loginHint := r.URL.Query().Get("login_hint")
@ -177,6 +179,18 @@ func authorizeOAuthPage(c *Context, w http.ResponseWriter, r *http.Request) {
return
}
// Validate PKCE requirements for public clients using authorization code flow
// Implicit flow doesn't require PKCE as it doesn't use code exchange
if oauthApp.IsPublicClient() && authRequest.ResponseType == model.AuthCodeResponseType && authRequest.CodeChallenge == "" {
err := model.NewAppError("authorizeOAuthPage", "api.oauth.allow_oauth.pkce_required_public.app_error", nil, "", http.StatusBadRequest)
utils.RenderWebError(c.App.Config(), w, r, err.StatusCode,
url.Values{
"type": []string{"oauth_pkce_required"},
"message": []string{"PKCE is required for public clients using authorization code flow"},
}, c.App.AsymmetricSigningKey())
return
}
isAuthorized := false
if _, err := c.App.GetPreferenceByCategoryAndNameForUser(c.AppContext, c.AppContext.Session().UserId, model.PreferenceCategoryAuthorizedOAuthApp, authRequest.ClientId); err == nil {
@ -244,10 +258,11 @@ func getAccessToken(c *Context, w http.ResponseWriter, r *http.Request) {
}
secret := r.FormValue("client_secret")
if secret == "" {
c.Err = model.NewAppError("getAccessToken", "api.oauth.get_access_token.bad_client_secret.app_error", nil, "", http.StatusBadRequest)
return
}
codeVerifier := r.FormValue("code_verifier")
// Authentication validation will be done at app layer based on client type
// For public clients: client_secret should be empty, code_verifier required
// For confidential clients: client_secret required, code_verifier optional but enforced if used
redirectURI := r.FormValue("redirect_uri")
@ -257,7 +272,7 @@ func getAccessToken(c *Context, w http.ResponseWriter, r *http.Request) {
auditRec.AddMeta("client_id", clientId)
c.LogAudit("attempt")
accessRsp, err := c.App.GetOAuthAccessTokenForCodeFlow(c.AppContext, clientId, grantType, redirectURI, code, secret, refreshToken)
accessRsp, err := c.App.GetOAuthAccessTokenForCodeFlow(c.AppContext, clientId, grantType, redirectURI, code, secret, refreshToken, codeVerifier)
if err != nil {
c.Err = err
return

View file

@ -67,6 +67,7 @@ func TestAuthorizeOAuthApp(t *testing.T) {
Description: "test",
CallbackUrls: []string{"https://nowhere.com"},
CreatorId: th.SystemAdminUser.Id,
ClientSecret: model.NewId(), // Explicitly set secret to make it confidential
}
rapp, appErr := th.App.CreateOAuthApp(oapp)
@ -148,6 +149,7 @@ func TestAuthorizeOAuthApp(t *testing.T) {
Description: "test",
CallbackUrls: []string{"https://nowhere.com?simply=lovely"},
CreatorId: th.SystemAdminUser.Id,
ClientSecret: model.NewId(), // Explicitly set secret to make it confidential
}
rapp, appErr = th.App.CreateOAuthApp(oappWithQueryParamInCallback)
@ -192,6 +194,7 @@ func TestDeauthorizeOAuthApp(t *testing.T) {
Description: "test",
CallbackUrls: []string{"https://nowhere.com"},
CreatorId: th.SystemAdminUser.Id,
ClientSecret: model.NewId(), // Explicitly set secret to make it confidential
}
rapp, appErr := th.App.CreateOAuthApp(oapp)
@ -251,6 +254,7 @@ func TestOAuthAccessToken(t *testing.T) {
Description: "test",
CallbackUrls: []string{"https://nowhere.com"},
CreatorId: th.SystemAdminUser.Id,
ClientSecret: model.NewId(), // Explicitly set secret to make it confidential
}
oauthApp, appErr := th.App.CreateOAuthApp(oauthApp)
require.Nil(t, appErr)
@ -521,9 +525,10 @@ func TestOAuthComplete(t *testing.T) {
th.AddPermissionToRole(model.PermissionManageOAuth.Id, model.SystemUserRoleId)
oauthApp := &model.OAuthApp{
Name: "TestApp5" + model.NewId(),
Homepage: "https://nowhere.com",
Description: "test",
Name: "TestApp5" + model.NewId(),
Homepage: "https://nowhere.com",
Description: "test",
ClientSecret: model.NewId(), // Explicitly set secret to make it confidential
CallbackUrls: []string{
apiClient.URL + "/signup/" + model.ServiceGitlab + "/complete",
apiClient.URL + "/login/" + model.ServiceGitlab + "/complete",
@ -889,3 +894,256 @@ func TestFullyQualifiedRedirectURL(t *testing.T) {
})
}
}
func TestAuthorizeOAuthPage(t *testing.T) {
th := Setup(t).InitBasic(t)
th.App.UpdateConfig(func(cfg *model.Config) {
*cfg.ServiceSettings.EnableOAuthServiceProvider = true
})
// Helper function to create context with session
createContext := func() *Context {
c := &Context{
App: th.App,
AppContext: th.Context,
Logger: th.TestLogger,
}
session := &model.Session{
UserId: th.BasicUser.Id,
Token: model.NewId(),
}
c.AppContext = c.AppContext.WithSession(session)
return c
}
t.Run("PublicClient_PKCERequired", func(t *testing.T) {
dcrRequest := &model.ClientRegistrationRequest{
ClientName: model.NewPointer("Public Client Test"),
RedirectURIs: []string{"https://example.com/callback"},
TokenEndpointAuthMethod: model.NewPointer(model.ClientAuthMethodNone),
ClientURI: model.NewPointer("https://example.com"),
}
publicApp, appErr := th.App.RegisterOAuthClient(th.Context, dcrRequest, th.BasicUser.Id)
require.Nil(t, appErr)
c := createContext()
responseWriter := httptest.NewRecorder()
requestURL := "/oauth/authorize?response_type=code&client_id=" + publicApp.Id + "&redirect_uri=" + url.QueryEscape(publicApp.CallbackUrls[0]) + "&state=test_state"
request := httptest.NewRequest(http.MethodGet, requestURL, nil)
authorizeOAuthPage(c, responseWriter, request)
assert.Contains(t, responseWriter.Body.String(), "PKCE+is+required+for+public+clients")
})
t.Run("PublicClient_WithPKCE_Success", func(t *testing.T) {
dcrRequest := &model.ClientRegistrationRequest{
ClientName: model.NewPointer("Public Client Test"),
RedirectURIs: []string{"https://example.com/callback"},
TokenEndpointAuthMethod: model.NewPointer(model.ClientAuthMethodNone),
ClientURI: model.NewPointer("https://example.com"),
}
publicApp, appErr := th.App.RegisterOAuthClient(th.Context, dcrRequest, th.BasicUser.Id)
require.Nil(t, appErr)
publicApp.IsTrusted = true
existingApp, getErr := th.App.GetOAuthApp(publicApp.Id)
require.Nil(t, getErr)
_, updateErr := th.App.UpdateOAuthApp(existingApp, publicApp)
require.Nil(t, updateErr)
c := createContext()
responseWriter := httptest.NewRecorder()
codeChallenge := "E9Melhoa2OwvFrEMTJguCHaoeK1t8URWbuGJSstw-cM"
codeChallengeMethod := model.PKCECodeChallengeMethodS256
request := httptest.NewRequest(http.MethodGet,
"/oauth/authorize?response_type=code&client_id="+publicApp.Id+"&redirect_uri="+url.QueryEscape(publicApp.CallbackUrls[0])+
"&state=test_state&code_challenge="+codeChallenge+"&code_challenge_method="+codeChallengeMethod, nil)
authorizeOAuthPage(c, responseWriter, request)
assert.Equal(t, http.StatusFound, responseWriter.Code)
location := responseWriter.Header().Get("Location")
assert.Contains(t, location, "https://example.com/callback")
assert.Contains(t, location, "code=")
assert.Contains(t, location, "state=test_state")
})
t.Run("ConfidentialClient_PKCEOptional", func(t *testing.T) {
confidentialApp := &model.OAuthApp{
Name: "Confidential Client Test",
CreatorId: th.BasicUser.Id,
Homepage: "https://example.com",
Description: "test confidential client",
CallbackUrls: []string{"https://example.com/callback"},
ClientSecret: "test-secret",
}
confidentialApp, appErr := th.App.CreateOAuthApp(confidentialApp)
require.Nil(t, appErr)
confidentialApp.IsTrusted = true
existingApp, getErr := th.App.GetOAuthApp(confidentialApp.Id)
require.Nil(t, getErr)
_, updateErr := th.App.UpdateOAuthApp(existingApp, confidentialApp)
require.Nil(t, updateErr)
c := createContext()
responseWriter := httptest.NewRecorder()
request := httptest.NewRequest(http.MethodGet,
"/oauth/authorize?response_type=code&client_id="+confidentialApp.Id+"&redirect_uri="+url.QueryEscape(confidentialApp.CallbackUrls[0])+"&state=test_state", nil)
authorizeOAuthPage(c, responseWriter, request)
assert.Equal(t, http.StatusFound, responseWriter.Code)
location := responseWriter.Header().Get("Location")
assert.Contains(t, location, "https://example.com/callback")
assert.Contains(t, location, "code=")
assert.Contains(t, location, "state=test_state")
})
}
func TestAuthorizeOAuthAppHandler(t *testing.T) {
th := Setup(t).InitBasic(t)
th.App.UpdateConfig(func(cfg *model.Config) {
*cfg.ServiceSettings.EnableOAuthServiceProvider = true
})
// Helper function to create context with session
createContext := func() *Context {
c := &Context{
App: th.App,
AppContext: th.Context,
Logger: th.TestLogger,
}
session := &model.Session{
UserId: th.BasicUser.Id,
Token: model.NewId(),
}
c.AppContext = c.AppContext.WithSession(session)
return c
}
t.Run("PublicClient_PKCEParameters", func(t *testing.T) {
dcrRequest := &model.ClientRegistrationRequest{
ClientName: model.NewPointer("Public Client Test"),
RedirectURIs: []string{"https://example.com/callback"},
TokenEndpointAuthMethod: model.NewPointer(model.ClientAuthMethodNone),
ClientURI: model.NewPointer("https://example.com"),
}
publicApp, appErr := th.App.RegisterOAuthClient(th.Context, dcrRequest, th.BasicUser.Id)
require.Nil(t, appErr)
c := createContext()
authRequest := &model.AuthorizeRequest{
ResponseType: model.AuthCodeResponseType,
ClientId: publicApp.Id,
RedirectURI: publicApp.CallbackUrls[0],
State: "test_state",
Scope: "user",
CodeChallenge: "E9Melhoa2OwvFrEMTJguCHaoeK1t8URWbuGJSstw-cM",
CodeChallengeMethod: model.PKCECodeChallengeMethodS256,
}
requestBodyBytes, err := json.Marshal(authRequest)
require.NoError(t, err)
requestBody := strings.NewReader(string(requestBodyBytes))
responseWriter := httptest.NewRecorder()
request := httptest.NewRequest(http.MethodPost, "/oauth/authorize", requestBody)
request.Header.Set("Content-Type", "application/json")
authorizeOAuthApp(c, responseWriter, request)
assert.Equal(t, http.StatusOK, responseWriter.Code)
var response map[string]string
err = json.Unmarshal(responseWriter.Body.Bytes(), &response)
require.NoError(t, err)
redirectURL := response["redirect"]
assert.NotEmpty(t, redirectURL)
assert.Contains(t, redirectURL, "code=")
})
}
func TestGetAccessToken(t *testing.T) {
th := Setup(t).InitBasic(t)
th.App.UpdateConfig(func(cfg *model.Config) {
*cfg.ServiceSettings.EnableOAuthServiceProvider = true
})
// Helper function to create context
createContext := func() *Context {
return &Context{
App: th.App,
AppContext: th.Context,
Logger: th.TestLogger,
}
}
t.Run("PublicClient_NoClientSecret", func(t *testing.T) {
dcrRequest := &model.ClientRegistrationRequest{
ClientName: model.NewPointer("Public Client Test"),
RedirectURIs: []string{"https://example.com/callback"},
TokenEndpointAuthMethod: model.NewPointer(model.ClientAuthMethodNone),
ClientURI: model.NewPointer("https://example.com"),
}
publicApp, appErr := th.App.RegisterOAuthClient(th.Context, dcrRequest, th.BasicUser.Id)
require.Nil(t, appErr)
codeVerifier := "dBjftJeZ4CVP-mB92K27uhbUJU1p1r_wW1gFWFOEjXk"
codeChallenge := "E9Melhoa2OwvFrEMTJguCHaoeK1t8URWbuGJSstw-cM"
authRequest := &model.AuthorizeRequest{
ResponseType: model.AuthCodeResponseType,
ClientId: publicApp.Id,
RedirectURI: publicApp.CallbackUrls[0],
State: "test_state",
Scope: "user",
CodeChallenge: codeChallenge,
CodeChallengeMethod: model.PKCECodeChallengeMethodS256,
}
redirectURL, appErr := th.App.AllowOAuthAppAccessToUser(th.Context, th.BasicUser.Id, authRequest)
require.Nil(t, appErr)
uri, err := url.Parse(redirectURL)
require.NoError(t, err)
code := uri.Query().Get("code")
require.NotEmpty(t, code)
c := createContext()
formData := url.Values{}
formData.Set("grant_type", model.AccessTokenGrantType)
formData.Set("code", code)
formData.Set("redirect_uri", publicApp.CallbackUrls[0])
formData.Set("client_id", publicApp.Id)
formData.Set("code_verifier", codeVerifier)
responseWriter := httptest.NewRecorder()
request := httptest.NewRequest(http.MethodPost, "/oauth/access_token", strings.NewReader(formData.Encode()))
request.Header.Set("Content-Type", "application/x-www-form-urlencoded")
getAccessToken(c, responseWriter, request)
assert.Equal(t, http.StatusOK, responseWriter.Code)
var tokenResponse model.AccessResponse
err = json.Unmarshal(responseWriter.Body.Bytes(), &tokenResponse)
require.NoError(t, err)
assert.NotEmpty(t, tokenResponse.AccessToken)
assert.Equal(t, model.AccessTokenType, tokenResponse.TokenType)
assert.Empty(t, tokenResponse.RefreshToken)
})
}

View file

@ -2472,6 +2472,10 @@
"id": "api.no_license",
"translation": "E10 or E20 license required to use this endpoint."
},
{
"id": "api.oauth.allow_oauth.pkce_required_public.app_error",
"translation": "PKCE (Proof Key for Code Exchange) is required for public OAuth clients using authorization code flow."
},
{
"id": "api.oauth.allow_oauth.redirect_callback.app_error",
"translation": "invalid_request: Supplied redirect_uri did not match registered callback_url."
@ -2512,10 +2516,6 @@
"id": "api.oauth.get_access_token.bad_client_id.app_error",
"translation": "invalid_request: Bad client_id."
},
{
"id": "api.oauth.get_access_token.bad_client_secret.app_error",
"translation": "invalid_request: Missing client_secret."
},
{
"id": "api.oauth.get_access_token.bad_grant.app_error",
"translation": "invalid_request: Bad grant_type."
@ -9024,6 +9024,26 @@
"id": "model.authorize.is_valid.client_id.app_error",
"translation": "Invalid client id."
},
{
"id": "model.authorize.is_valid.code_challenge.app_error",
"translation": "Code challenge is required when using PKCE."
},
{
"id": "model.authorize.is_valid.code_challenge.format.app_error",
"translation": "Code challenge must be base64url-encoded (using characters A-Z, a-z, 0-9, -, _)."
},
{
"id": "model.authorize.is_valid.code_challenge.length.app_error",
"translation": "Code challenge must be between 43 and 128 characters long."
},
{
"id": "model.authorize.is_valid.code_challenge_method.app_error",
"translation": "Code challenge method is required when using PKCE."
},
{
"id": "model.authorize.is_valid.code_challenge_method.unsupported.app_error",
"translation": "Only 'S256' code challenge method is supported."
},
{
"id": "model.authorize.is_valid.create_at.app_error",
"translation": "Create at must be a valid time."
@ -9052,6 +9072,22 @@
"id": "model.authorize.is_valid.user_id.app_error",
"translation": "Invalid user id."
},
{
"id": "model.authorize.validate_pkce.not_used_in_auth.app_error",
"translation": "PKCE code verifier provided but PKCE was not used during authorization."
},
{
"id": "model.authorize.validate_pkce.public_client_required.app_error",
"translation": "PKCE (Proof Key for Code Exchange) is required for public clients."
},
{
"id": "model.authorize.validate_pkce.verification_failed.app_error",
"translation": "PKCE code verifier does not match the code challenge."
},
{
"id": "model.authorize.validate_pkce.verifier_required.app_error",
"translation": "PKCE code verifier is required when PKCE is used."
},
{
"id": "model.bot.is_valid.create_at.app_error",
"translation": "Invalid create at."
@ -10292,6 +10328,22 @@
"id": "model.oauth.is_valid.update_at.app_error",
"translation": "Update at must be a valid time."
},
{
"id": "model.oauth.validate_grant.credentials.app_error",
"translation": "Invalid client credentials."
},
{
"id": "model.oauth.validate_grant.pkce_required.app_error",
"translation": "PKCE (Proof Key for Code Exchange) is required for public clients."
},
{
"id": "model.oauth.validate_grant.public_client_refresh_token.app_error",
"translation": "Public clients cannot use refresh token grant type."
},
{
"id": "model.oauth.validate_grant.public_client_secret.app_error",
"translation": "Public clients must not provide a client secret."
},
{
"id": "model.outgoing_hook.icon_url.app_error",
"translation": "Invalid icon."

View file

@ -4,33 +4,50 @@
package model
import (
"crypto/sha256"
"encoding/base64"
"net/http"
"regexp"
)
var (
codeChallengeRegex = regexp.MustCompile("^[A-Za-z0-9_-]+$")
codeVerifierRegex = regexp.MustCompile(`^[A-Za-z0-9\-._~]+$`)
)
const (
AuthCodeExpireTime = 60 * 10 // 10 minutes
AuthCodeResponseType = "code"
ImplicitResponseType = "token"
DefaultScope = "user"
AuthCodeExpireTime = 60 * 10 // 10 minutes
AuthCodeResponseType = "code"
ImplicitResponseType = "token"
DefaultScope = "user"
PKCECodeChallengeMethodS256 = "S256"
PKCECodeChallengeMinLength = 43
PKCECodeChallengeMaxLength = 128
PKCECodeVerifierMinLength = 43
PKCECodeVerifierMaxLength = 128
)
type AuthData struct {
ClientId string `json:"client_id"`
UserId string `json:"user_id"`
Code string `json:"code"`
ExpiresIn int32 `json:"expires_in"`
CreateAt int64 `json:"create_at"`
RedirectUri string `json:"redirect_uri"`
State string `json:"state"`
Scope string `json:"scope"`
ClientId string `json:"client_id"`
UserId string `json:"user_id"`
Code string `json:"code"`
ExpiresIn int32 `json:"expires_in"`
CreateAt int64 `json:"create_at"`
RedirectUri string `json:"redirect_uri"`
State string `json:"state"`
Scope string `json:"scope"`
CodeChallenge string `json:"code_challenge,omitempty"`
CodeChallengeMethod string `json:"code_challenge_method,omitempty"`
}
type AuthorizeRequest struct {
ResponseType string `json:"response_type"`
ClientId string `json:"client_id"`
RedirectURI string `json:"redirect_uri"`
Scope string `json:"scope"`
State string `json:"state"`
ResponseType string `json:"response_type"`
ClientId string `json:"client_id"`
RedirectURI string `json:"redirect_uri"`
Scope string `json:"scope"`
State string `json:"state"`
CodeChallenge string `json:"code_challenge,omitempty"`
CodeChallengeMethod string `json:"code_challenge_method,omitempty"`
}
// IsValid validates the AuthData and returns an error if it isn't configured
@ -68,6 +85,13 @@ func (ad *AuthData) IsValid() *AppError {
return NewAppError("AuthData.IsValid", "model.authorize.is_valid.scope.app_error", nil, "client_id="+ad.ClientId, http.StatusBadRequest)
}
// PKCE validation - if one PKCE field is present, both must be present and valid
if ad.CodeChallenge != "" || ad.CodeChallengeMethod != "" {
if err := ad.validatePKCE(); err != nil {
return err
}
}
return nil
}
@ -94,6 +118,13 @@ func (ar *AuthorizeRequest) IsValid() *AppError {
return NewAppError("AuthData.IsValid", "model.authorize.is_valid.scope.app_error", nil, "client_id="+ar.ClientId, http.StatusBadRequest)
}
// PKCE validation - if one PKCE field is present, both must be present and valid
if ar.CodeChallenge != "" || ar.CodeChallengeMethod != "" {
if err := ar.validatePKCE(); err != nil {
return err
}
}
return nil
}
@ -114,3 +145,109 @@ func (ad *AuthData) PreSave() {
func (ad *AuthData) IsExpired() bool {
return GetMillis() > ad.CreateAt+int64(ad.ExpiresIn*1000)
}
// validatePKCEParameters validates PKCE parameters (shared validation logic)
func validatePKCEParameters(codeChallenge, codeChallengeMethod, clientId, caller string) *AppError {
if codeChallenge == "" {
return NewAppError(caller, "model.authorize.is_valid.code_challenge.app_error", nil, "client_id="+clientId, http.StatusBadRequest)
}
if codeChallengeMethod == "" {
return NewAppError(caller, "model.authorize.is_valid.code_challenge_method.app_error", nil, "client_id="+clientId, http.StatusBadRequest)
}
// Only support S256 method for security
if codeChallengeMethod != PKCECodeChallengeMethodS256 {
return NewAppError(caller, "model.authorize.is_valid.code_challenge_method.unsupported.app_error", nil, "client_id="+clientId+", method="+codeChallengeMethod, http.StatusBadRequest)
}
// Validate code challenge format (base64url encoded)
if len(codeChallenge) < PKCECodeChallengeMinLength || len(codeChallenge) > PKCECodeChallengeMaxLength {
return NewAppError(caller, "model.authorize.is_valid.code_challenge.length.app_error", nil, "client_id="+clientId, http.StatusBadRequest)
}
// Validate base64url format (no padding, URL-safe characters)
if !codeChallengeRegex.MatchString(codeChallenge) {
return NewAppError(caller, "model.authorize.is_valid.code_challenge.format.app_error", nil, "client_id="+clientId, http.StatusBadRequest)
}
return nil
}
// validatePKCE validates PKCE parameters for AuthData
func (ad *AuthData) validatePKCE() *AppError {
return validatePKCEParameters(ad.CodeChallenge, ad.CodeChallengeMethod, ad.ClientId, "AuthData.validatePKCE")
}
// validatePKCE validates PKCE parameters for AuthorizeRequest
func (ar *AuthorizeRequest) validatePKCE() *AppError {
return validatePKCEParameters(ar.CodeChallenge, ar.CodeChallengeMethod, ar.ClientId, "AuthorizeRequest.validatePKCE")
}
// VerifyPKCE verifies a PKCE code_verifier against the stored code_challenge
func (ad *AuthData) VerifyPKCE(codeVerifier string) bool {
// Both empty = no PKCE was used (backward compatibility)
if ad.CodeChallenge == "" && ad.CodeChallengeMethod == "" {
return true
}
// Only one empty = invalid data state
if ad.CodeChallenge == "" || ad.CodeChallengeMethod == "" {
return false
}
// Validate code verifier length
if len(codeVerifier) < PKCECodeVerifierMinLength || len(codeVerifier) > PKCECodeVerifierMaxLength {
return false
}
// Validate code verifier format (unreserved characters from RFC 3986)
if !codeVerifierRegex.MatchString(codeVerifier) {
return false
}
// Only S256 method is supported
if ad.CodeChallengeMethod != PKCECodeChallengeMethodS256 {
return false
}
// Calculate S256 challenge: BASE64URL-ENCODE(SHA256(ASCII(code_verifier)))
hash := sha256.Sum256([]byte(codeVerifier))
calculatedChallenge := base64.RawURLEncoding.EncodeToString(hash[:])
return calculatedChallenge == ad.CodeChallenge
}
// ValidatePKCEForClientType validates PKCE parameters based on OAuth client type and security requirements
func (ad *AuthData) ValidatePKCEForClientType(isPublicClient bool, codeVerifier string) *AppError {
if isPublicClient {
// RFC 7636: Public clients MUST use PKCE
if ad.CodeChallenge == "" {
return NewAppError("AuthData.ValidatePKCEForClientType", "model.authorize.validate_pkce.public_client_required.app_error", nil, "client_id="+ad.ClientId, http.StatusBadRequest)
}
if codeVerifier == "" {
return NewAppError("AuthData.ValidatePKCEForClientType", "model.authorize.validate_pkce.verifier_required.app_error", nil, "client_id="+ad.ClientId, http.StatusBadRequest)
}
// Verify the code verifier matches the stored code challenge
if !ad.VerifyPKCE(codeVerifier) {
return NewAppError("AuthData.ValidatePKCEForClientType", "model.authorize.validate_pkce.verification_failed.app_error", nil, "client_id="+ad.ClientId, http.StatusBadRequest)
}
} else {
// Confidential clients: PKCE is optional but enforced if initiated
if ad.CodeChallenge != "" {
// Client started flow with PKCE - code_verifier is required
if codeVerifier == "" {
return NewAppError("AuthData.ValidatePKCEForClientType", "model.authorize.validate_pkce.verifier_required.app_error", nil, "client_id="+ad.ClientId, http.StatusBadRequest)
}
// Verify the code verifier matches the stored code challenge
if !ad.VerifyPKCE(codeVerifier) {
return NewAppError("AuthData.ValidatePKCEForClientType", "model.authorize.validate_pkce.verification_failed.app_error", nil, "client_id="+ad.ClientId, http.StatusBadRequest)
}
} else if codeVerifier != "" {
// Client provided code_verifier but didn't use PKCE in authorization - reject
return NewAppError("AuthData.ValidatePKCEForClientType", "model.authorize.validate_pkce.not_used_in_auth.app_error", nil, "client_id="+ad.ClientId, http.StatusBadRequest)
}
}
return nil
}

View file

@ -80,3 +80,138 @@ func TestAuthIsValid(t *testing.T) {
ad.RedirectUri = "http://example.com"
require.Nil(t, ad.IsValid())
}
func TestAuthDataVerifyPKCE(t *testing.T) {
t.Run("S256_ValidVerifier", func(t *testing.T) {
codeVerifier := "dBjftJeZ4CVP-mB92K27uhbUJU1p1r_wW1gFWFOEjXk"
expectedChallenge := "E9Melhoa2OwvFrEMTJguCHaoeK1t8URWbuGJSstw-cM"
authData := &AuthData{
CodeChallenge: expectedChallenge,
CodeChallengeMethod: PKCECodeChallengeMethodS256,
}
require.True(t, authData.VerifyPKCE(codeVerifier))
})
t.Run("S256_InvalidVerifier", func(t *testing.T) {
authData := &AuthData{
CodeChallenge: "E9Melhoa2OwvFrEMTJguCHaoeK1t8URWbuGJSstw-cM",
CodeChallengeMethod: PKCECodeChallengeMethodS256,
}
require.False(t, authData.VerifyPKCE("wrong_verifier"))
require.False(t, authData.VerifyPKCE(""))
require.False(t, authData.VerifyPKCE("short"))
longVerifier := make([]byte, PKCECodeVerifierMaxLength+1)
for i := range longVerifier {
longVerifier[i] = 'a'
}
require.False(t, authData.VerifyPKCE(string(longVerifier)))
require.False(t, authData.VerifyPKCE("invalid@characters#here!"))
})
t.Run("BackwardCompatibility", func(t *testing.T) {
authData := &AuthData{}
require.True(t, authData.VerifyPKCE("any_verifier"))
require.True(t, authData.VerifyPKCE(""))
})
t.Run("UnsupportedMethod", func(t *testing.T) {
authData := &AuthData{
CodeChallenge: "some_challenge",
CodeChallengeMethod: "plain",
}
require.False(t, authData.VerifyPKCE("any_verifier"))
})
}
func TestValidatePKCEParameters(t *testing.T) {
validChallenge := "E9Melhoa2OwvFrEMTJguCHaoeK1t8URWbuGJSstw-cM"
validMethod := PKCECodeChallengeMethodS256
clientId := NewId()
t.Run("ValidParameters", func(t *testing.T) {
require.Nil(t, validatePKCEParameters(validChallenge, validMethod, clientId, "test"))
})
t.Run("EmptyChallenge", func(t *testing.T) {
require.NotNil(t, validatePKCEParameters("", validMethod, clientId, "test"))
})
t.Run("EmptyMethod", func(t *testing.T) {
require.NotNil(t, validatePKCEParameters(validChallenge, "", clientId, "test"))
})
t.Run("InvalidMethod", func(t *testing.T) {
require.NotNil(t, validatePKCEParameters(validChallenge, "plain", clientId, "test"))
})
t.Run("ChallengeTooShort", func(t *testing.T) {
require.NotNil(t, validatePKCEParameters("short", validMethod, clientId, "test"))
})
t.Run("ChallengeTooLong", func(t *testing.T) {
longChallenge := make([]byte, PKCECodeChallengeMaxLength+1)
for i := range longChallenge {
longChallenge[i] = 'a'
}
require.NotNil(t, validatePKCEParameters(string(longChallenge), validMethod, clientId, "test"))
})
}
func TestAuthorizeRequestPKCE(t *testing.T) {
t.Run("RequiredTogether", func(t *testing.T) {
authRequest := &AuthorizeRequest{
ResponseType: ResponseTypeCode,
ClientId: NewId(),
RedirectURI: "https://example.com/callback",
State: "test_state",
Scope: "user",
}
require.Nil(t, authRequest.IsValid())
authRequest.CodeChallenge = "E9Melhoa2OwvFrEMTJguCHaoeK1t8URWbuGJSstw-cM"
require.NotNil(t, authRequest.IsValid())
authRequest.CodeChallengeMethod = PKCECodeChallengeMethodS256
require.Nil(t, authRequest.IsValid())
authRequest.CodeChallenge = ""
require.NotNil(t, authRequest.IsValid())
})
t.Run("ValidationDetails", func(t *testing.T) {
authRequest := &AuthorizeRequest{
ResponseType: ResponseTypeCode,
ClientId: NewId(),
RedirectURI: "https://example.com/callback",
State: "test_state",
Scope: "user",
CodeChallenge: "E9Melhoa2OwvFrEMTJguCHaoeK1t8URWbuGJSstw-cM",
CodeChallengeMethod: PKCECodeChallengeMethodS256,
}
require.Nil(t, authRequest.IsValid())
authRequest.CodeChallengeMethod = "plain"
require.NotNil(t, authRequest.IsValid())
authRequest.CodeChallengeMethod = PKCECodeChallengeMethodS256
authRequest.CodeChallenge = "short"
require.NotNil(t, authRequest.IsValid())
longChallenge := make([]byte, PKCECodeChallengeMaxLength+1)
for i := range longChallenge {
longChallenge[i] = 'a'
}
authRequest.CodeChallenge = string(longChallenge)
require.NotNil(t, authRequest.IsValid())
})
}

View file

@ -4,6 +4,7 @@
package model
import (
"crypto/subtle"
"fmt"
"net/http"
"slices"
@ -70,7 +71,8 @@ func (a *OAuthApp) IsValid() *AppError {
return NewAppError("OAuthApp.IsValid", "model.oauth.is_valid.creator_id.app_error", nil, "app_id="+a.Id, http.StatusBadRequest)
}
if a.ClientSecret == "" || len(a.ClientSecret) > 128 {
// Validate client secret length if present
if a.ClientSecret != "" && len(a.ClientSecret) > 128 {
return NewAppError("OAuthApp.IsValid", "model.oauth.is_valid.client_secret.app_error", nil, "app_id="+a.Id, http.StatusBadRequest)
}
@ -120,9 +122,8 @@ func (a *OAuthApp) PreSave() {
a.Id = NewId()
}
if a.ClientSecret == "" {
a.ClientSecret = NewId()
}
// PreSave no longer generates client secrets - callers must explicitly set ClientSecret
// if they want to create a confidential client
a.CreateAt = GetMillis()
a.UpdateAt = a.CreateAt
@ -147,7 +148,8 @@ func (a *OAuthApp) IsValidRedirectURL(url string) bool {
return slices.Contains(a.CallbackUrls, url)
}
// GetTokenEndpointAuthMethod returns the token endpoint auth method based on whether the app has a client secret
// GetTokenEndpointAuthMethod returns the OAuth token endpoint authentication method
// based on whether the client has a secret
func (a *OAuthApp) GetTokenEndpointAuthMethod() string {
if a.ClientSecret == "" {
return ClientAuthMethodNone
@ -155,6 +157,49 @@ func (a *OAuthApp) GetTokenEndpointAuthMethod() string {
return ClientAuthMethodClientSecretPost
}
// IsPublicClient returns true if this is a public client (uses "none" auth method)
func (a *OAuthApp) IsPublicClient() bool {
return a.GetTokenEndpointAuthMethod() == ClientAuthMethodNone
}
// ValidateForGrantType validates the OAuth app for a specific grant type and provided credentials
func (a *OAuthApp) ValidateForGrantType(grantType, clientSecret, codeVerifier string) *AppError {
if a.IsPublicClient() {
return a.validatePublicClientGrant(grantType, clientSecret, codeVerifier)
}
return a.validateConfidentialClientGrant(grantType, clientSecret)
}
// validatePublicClientGrant validates that public client requests follow OAuth 2.1 security requirements
func (a *OAuthApp) validatePublicClientGrant(grantType, clientSecret, codeVerifier string) *AppError {
// Public clients must not provide a client secret
if clientSecret != "" {
return NewAppError("OAuthApp.validatePublicClientGrant", "model.oauth.validate_grant.public_client_secret.app_error", nil, "app_id="+a.Id, http.StatusBadRequest)
}
// Public clients cannot use refresh token grant type
if grantType == RefreshTokenGrantType {
return NewAppError("OAuthApp.validatePublicClientGrant", "model.oauth.validate_grant.public_client_refresh_token.app_error", nil, "app_id="+a.Id, http.StatusBadRequest)
}
// Public clients must use PKCE for authorization code grant
if grantType == AccessTokenGrantType && codeVerifier == "" {
return NewAppError("OAuthApp.validatePublicClientGrant", "model.oauth.validate_grant.pkce_required.app_error", nil, "app_id="+a.Id, http.StatusBadRequest)
}
return nil
}
// validateConfidentialClientGrant validates confidential client authentication
func (a *OAuthApp) validateConfidentialClientGrant(grantType, clientSecret string) *AppError {
// Confidential clients must provide correct client secret
if subtle.ConstantTimeCompare([]byte(a.ClientSecret), []byte(clientSecret)) == 0 {
return NewAppError("OAuthApp.validateConfidentialClientGrant", "model.oauth.validate_grant.credentials.app_error", nil, "app_id="+a.Id, http.StatusUnauthorized)
}
return nil
}
func NewOAuthAppFromClientRegistration(req *ClientRegistrationRequest, creatorId string) *OAuthApp {
app := &OAuthApp{
CreatorId: creatorId,
@ -168,6 +213,16 @@ func NewOAuthAppFromClientRegistration(req *ClientRegistrationRequest, creatorId
app.Name = "Dynamically Registered Client"
}
// Generate client secret based on requested auth method, default to confidential client
requestedAuthMethod := ClientAuthMethodClientSecretPost
if req.TokenEndpointAuthMethod != nil {
requestedAuthMethod = *req.TokenEndpointAuthMethod
}
if requestedAuthMethod != ClientAuthMethodNone {
app.ClientSecret = NewId()
}
if req.ClientURI != nil {
app.Homepage = *req.ClientURI
}
@ -176,16 +231,16 @@ func NewOAuthAppFromClientRegistration(req *ClientRegistrationRequest, creatorId
}
func (a *OAuthApp) ToClientRegistrationResponse(siteURL string) *ClientRegistrationResponse {
authMethod := a.GetTokenEndpointAuthMethod()
resp := &ClientRegistrationResponse{
ClientID: a.Id,
RedirectURIs: a.CallbackUrls,
TokenEndpointAuthMethod: authMethod,
TokenEndpointAuthMethod: a.GetTokenEndpointAuthMethod(),
GrantTypes: GetDefaultGrantTypes(),
ResponseTypes: GetDefaultResponseTypes(),
Scope: ScopeUser,
}
if authMethod != ClientAuthMethodNone {
if !a.IsPublicClient() {
resp.ClientSecret = &a.ClientSecret
}

View file

@ -21,6 +21,7 @@ type ClientRegistrationResponse struct {
TokenEndpointAuthMethod string `json:"token_endpoint_auth_method"`
GrantTypes []string `json:"grant_types"`
ResponseTypes []string `json:"response_types"`
Scope string `json:"scope,omitempty"`
ClientName *string `json:"client_name,omitempty"`
ClientURI *string `json:"client_uri,omitempty"`
}
@ -60,7 +61,7 @@ func (r *ClientRegistrationRequest) IsValid() *AppError {
}
}
if r.TokenEndpointAuthMethod != nil && *r.TokenEndpointAuthMethod != ClientAuthMethodClientSecretPost {
if r.TokenEndpointAuthMethod != nil && *r.TokenEndpointAuthMethod != ClientAuthMethodClientSecretPost && *r.TokenEndpointAuthMethod != ClientAuthMethodNone {
return NewAppError("ClientRegistrationRequest.IsValid", "model.dcr.is_valid.unsupported_auth_method.app_error", nil, "method="+*r.TokenEndpointAuthMethod, http.StatusBadRequest)
}

View file

@ -0,0 +1,77 @@
// Copyright (c) 2015-present Mattermost, Inc. All Rights Reserved.
// See LICENSE.txt for license information.
package model
import (
"testing"
"github.com/stretchr/testify/require"
)
func TestClientRegistrationRequestIsValid(t *testing.T) {
t.Run("PublicClient_Valid", func(t *testing.T) {
req := &ClientRegistrationRequest{
RedirectURIs: []string{"https://example.com/callback"},
TokenEndpointAuthMethod: NewPointer(ClientAuthMethodNone),
ClientName: NewPointer("Test Public Client"),
}
require.Nil(t, req.IsValid())
})
t.Run("PublicClient_AuthMethodValidation", func(t *testing.T) {
req := &ClientRegistrationRequest{
RedirectURIs: []string{"https://example.com/callback"},
TokenEndpointAuthMethod: NewPointer(ClientAuthMethodNone),
ClientName: NewPointer("Test Public Client"),
}
require.Nil(t, req.IsValid())
req.TokenEndpointAuthMethod = NewPointer("invalid_method")
require.NotNil(t, req.IsValid())
})
t.Run("PublicClient_RedirectURIValidation", func(t *testing.T) {
req := &ClientRegistrationRequest{
TokenEndpointAuthMethod: NewPointer(ClientAuthMethodNone),
ClientName: NewPointer("Test Public Client"),
}
require.NotNil(t, req.IsValid())
req.RedirectURIs = []string{"https://example.com/callback"}
require.Nil(t, req.IsValid())
req.RedirectURIs = []string{"http://localhost:3000/callback"}
require.Nil(t, req.IsValid())
req.RedirectURIs = []string{"invalid-uri"}
require.NotNil(t, req.IsValid())
})
}
func TestNewOAuthAppFromClientRegistration(t *testing.T) {
t.Run("PublicClient", func(t *testing.T) {
req := &ClientRegistrationRequest{
RedirectURIs: []string{"https://example.com/callback"},
TokenEndpointAuthMethod: NewPointer(ClientAuthMethodNone),
ClientName: NewPointer("Test Public Client"),
}
creatorId := NewId()
app := NewOAuthAppFromClientRegistration(req, creatorId)
require.Equal(t, creatorId, app.CreatorId)
require.Equal(t, req.RedirectURIs, []string(app.CallbackUrls))
require.Equal(t, *req.TokenEndpointAuthMethod, app.GetTokenEndpointAuthMethod())
require.Equal(t, *req.ClientName, app.Name)
require.True(t, app.IsDynamicallyRegistered)
app.PreSave()
require.Nil(t, app.IsValid())
require.Empty(t, app.ClientSecret)
})
}

View file

@ -14,6 +14,7 @@ type AuthorizationServerMetadata struct {
ScopesSupported []string `json:"scopes_supported,omitempty"`
GrantTypesSupported []string `json:"grant_types_supported,omitempty"`
TokenEndpointAuthMethodsSupported []string `json:"token_endpoint_auth_methods_supported,omitempty"`
CodeChallengeMethodsSupported []string `json:"code_challenge_methods_supported,omitempty"`
}
const (
@ -62,5 +63,8 @@ func GetDefaultMetadata(siteURL string) (*AuthorizationServerMetadata, error) {
ScopesSupported: []string{
ScopeUser,
},
CodeChallengeMethodsSupported: []string{
PKCECodeChallengeMethodS256, // S256 method supported for optional PKCE
},
}, nil
}

View file

@ -10,16 +10,35 @@ import (
)
func TestOAuthAppPreSave(t *testing.T) {
a1 := OAuthApp{}
a1.Id = NewId()
a1.Name = "TestOAuthApp" + NewId()
a1.CallbackUrls = []string{"https://nowhere.com"}
a1.Homepage = "https://nowhere.com"
a1.IconURL = "https://nowhere.com/icon_image.png"
a1.ClientSecret = NewId()
a1.PreSave()
a1.Etag()
a1.Sanitize()
t.Run("WithClientSecret", func(t *testing.T) {
a1 := OAuthApp{}
a1.Id = NewId()
a1.Name = "TestOAuthApp" + NewId()
a1.CallbackUrls = []string{"https://nowhere.com"}
a1.Homepage = "https://nowhere.com"
a1.IconURL = "https://nowhere.com/icon_image.png"
a1.ClientSecret = NewId()
a1.PreSave()
a1.Etag()
a1.Sanitize()
})
t.Run("NoSecretGeneration", func(t *testing.T) {
app := OAuthApp{
Name: "Test Client",
CallbackUrls: []string{"https://example.com/callback"},
Homepage: "https://example.com",
}
app.PreSave()
require.Empty(t, app.ClientSecret)
require.NotEmpty(t, app.Id)
require.NotZero(t, app.CreateAt)
require.NotZero(t, app.UpdateAt)
require.True(t, app.IsPublicClient())
require.Equal(t, ClientAuthMethodNone, app.GetTokenEndpointAuthMethod())
})
}
func TestOAuthAppPreUpdate(t *testing.T) {
@ -34,37 +53,72 @@ func TestOAuthAppPreUpdate(t *testing.T) {
}
func TestOAuthAppIsValid(t *testing.T) {
app := OAuthApp{}
t.Run("RequiredFields", func(t *testing.T) {
app := OAuthApp{}
require.NotNil(t, app.IsValid())
require.NotNil(t, app.IsValid())
app.Id = NewId()
require.NotNil(t, app.IsValid())
app.Id = NewId()
require.NotNil(t, app.IsValid())
app.CreateAt = 1
require.NotNil(t, app.IsValid())
app.CreateAt = 1
require.NotNil(t, app.IsValid())
app.UpdateAt = 1
require.NotNil(t, app.IsValid())
app.UpdateAt = 1
require.NotNil(t, app.IsValid())
app.CreatorId = NewId()
require.NotNil(t, app.IsValid())
app.CreatorId = NewId()
require.NotNil(t, app.IsValid())
app.ClientSecret = NewId()
require.NotNil(t, app.IsValid())
app.ClientSecret = NewId()
require.NotNil(t, app.IsValid())
app.Name = "TestOAuthApp"
require.NotNil(t, app.IsValid())
app.Name = "TestOAuthApp"
require.NotNil(t, app.IsValid())
app.CallbackUrls = []string{"https://nowhere.com"}
require.NotNil(t, app.IsValid())
app.CallbackUrls = []string{"https://nowhere.com"}
require.NotNil(t, app.IsValid())
app.MattermostAppID = "Some app ID"
require.NotNil(t, app.IsValid())
app.MattermostAppID = "Some app ID"
require.NotNil(t, app.IsValid())
app.Homepage = "https://nowhere.com"
require.Nil(t, app.IsValid())
app.Homepage = "https://nowhere.com"
require.Nil(t, app.IsValid())
app.IconURL = "https://nowhere.com/icon_image.png"
require.Nil(t, app.IsValid())
app.IconURL = "https://nowhere.com/icon_image.png"
require.Nil(t, app.IsValid())
})
t.Run("PublicClient", func(t *testing.T) {
app := OAuthApp{
Id: NewId(),
CreatorId: NewId(),
CreateAt: 1,
UpdateAt: 1,
Name: "Test Public Client",
CallbackUrls: []string{"https://example.com/callback"},
Homepage: "https://example.com",
}
require.Nil(t, app.IsValid())
require.True(t, app.IsPublicClient())
require.Equal(t, ClientAuthMethodNone, app.GetTokenEndpointAuthMethod())
})
t.Run("ConfidentialClient", func(t *testing.T) {
app := OAuthApp{
Id: NewId(),
CreatorId: NewId(),
CreateAt: 1,
UpdateAt: 1,
Name: "Test Confidential Client",
CallbackUrls: []string{"https://example.com/callback"},
Homepage: "https://example.com",
ClientSecret: NewId(),
}
require.Nil(t, app.IsValid())
require.False(t, app.IsPublicClient())
require.Equal(t, ClientAuthMethodClientSecretPost, app.GetTokenEndpointAuthMethod())
})
}

View file

@ -168,10 +168,10 @@ export function getOAuthAppInfo(clientId) {
* @param {*}
* @returns {ActionResult<{redirect: string}>}
*/
export function allowOAuth2({responseType, clientId, redirectUri, state, scope}) {
export function allowOAuth2({responseType, clientId, redirectUri, state, scope, codeChallenge, codeChallengeMethod}) {
return bindClientFunc({
clientFunc: Client4.authorizeOAuthApp,
params: [responseType, clientId, redirectUri, state, scope],
params: [responseType, clientId, redirectUri, state, scope, codeChallenge, codeChallengeMethod],
});
}

View file

@ -60,6 +60,8 @@ describe('components/user_settings/display/UserSettingsDisplay', () => {
const expected = {
clientId: '1234abcd',
codeChallenge: null,
codeChallengeMethod: null,
responseType: null,
redirectUri: null,
state: null,

View file

@ -71,6 +71,8 @@ export default class Authorize extends React.PureComponent<Props, State> {
redirectUri: searchParams.get('redirect_uri'),
state: searchParams.get('state'),
scope: searchParams.get('store'),
codeChallenge: searchParams.get('code_challenge'),
codeChallengeMethod: searchParams.get('code_challenge_method'),
};
this.props.actions.allowOAuth2(params).then(

View file

@ -1180,10 +1180,20 @@ export default class Client4 {
);
};
authorizeOAuthApp = (responseType: string, clientId: string, redirectUri: string, state: string, scope: string) => {
authorizeOAuthApp = (responseType: string, clientId: string, redirectUri: string, state: string, scope: string, codeChallenge?: string, codeChallengeMethod?: string) => {
const body: any = {client_id: clientId, response_type: responseType, redirect_uri: redirectUri, state, scope};
// Include PKCE parameters if provided
if (codeChallenge) {
body.code_challenge = codeChallenge;
}
if (codeChallengeMethod) {
body.code_challenge_method = codeChallengeMethod;
}
return this.doFetch<void>(
`${this.url}/oauth/authorize`,
{method: 'post', body: JSON.stringify({client_id: clientId, response_type: responseType, redirect_uri: redirectUri, state, scope})},
{method: 'post', body: JSON.stringify(body)},
);
};