mirror of
https://github.com/pocket-id/pocket-id.git
synced 2026-10-10 03:39:05 +02:00
Merge remote-tracking branch 'origin/main' into claude/merge-conflicts-reviewer-comments-ltre0t
# Conflicts: # backend/internal/service/e2etest_service.go
This commit is contained in:
@@ -451,6 +451,7 @@ func (oc *OidcController) updateAllowedUserGroupsHandler(c *gin.Context) error {
|
||||
// @Param pagination[limit] query int false "Number of items per page" default(20)
|
||||
// @Param sort[column] query string false "Column to sort by"
|
||||
// @Param sort[direction] query string false "Sort direction (asc or desc)" default("asc")
|
||||
// @Param filters[hasLaunchURL] query bool false "Filter clients by whether a launch URL is configured"
|
||||
// @Success 200 {object} dto.Paginated[dto.AuthorizedOidcClientDto]
|
||||
// @Router /api/oidc/users/me/authorized-clients [get]
|
||||
func (oc *OidcController) listOwnAuthorizedClientsHandler(c *gin.Context) error {
|
||||
@@ -467,6 +468,7 @@ func (oc *OidcController) listOwnAuthorizedClientsHandler(c *gin.Context) error
|
||||
// @Param pagination[limit] query int false "Number of items per page" default(20)
|
||||
// @Param sort[column] query string false "Column to sort by"
|
||||
// @Param sort[direction] query string false "Sort direction (asc or desc)" default("asc")
|
||||
// @Param filters[hasLaunchURL] query bool false "Filter clients by whether a launch URL is configured"
|
||||
// @Success 200 {object} dto.Paginated[dto.AuthorizedOidcClientDto]
|
||||
// @Router /api/oidc/users/{id}/authorized-clients [get]
|
||||
func (oc *OidcController) listAuthorizedClientsHandler(c *gin.Context) error {
|
||||
@@ -524,6 +526,7 @@ func (oc *OidcController) revokeOwnClientAuthorizationHandler(c *gin.Context) er
|
||||
// @Param pagination[limit] query int false "Number of items per page" default(20)
|
||||
// @Param sort[column] query string false "Column to sort by"
|
||||
// @Param sort[direction] query string false "Sort direction (asc or desc)" default("asc")
|
||||
// @Param filters[hasLaunchURL] query bool false "Filter clients by whether a launch URL is configured"
|
||||
// @Success 200 {object} dto.Paginated[dto.AccessibleOidcClientDto]
|
||||
// @Router /api/oidc/users/me/clients [get]
|
||||
func (oc *OidcController) listOwnAccessibleClientsHandler(c *gin.Context) error {
|
||||
|
||||
@@ -6,6 +6,7 @@ import (
|
||||
"fmt"
|
||||
"net/http"
|
||||
"net/url"
|
||||
"slices"
|
||||
"strings"
|
||||
|
||||
"github.com/ory/fosite"
|
||||
@@ -129,23 +130,9 @@ func (cimdPolicy) ValidateCIMDClient(_ context.Context, doc *fosite.ClientMetada
|
||||
return fmt.Errorf("client metadata documents only support token_endpoint_auth_method %q, got %q", "none", doc.TokenEndpointAuthMethod)
|
||||
}
|
||||
|
||||
// Restrict metadata clients to grant types implemented by Pocket ID and require a flow that can initiate authorization
|
||||
grantTypes := doc.GrantTypes
|
||||
if len(grantTypes) == 0 {
|
||||
grantTypes = []string{string(fosite.GrantTypeAuthorizationCode)}
|
||||
}
|
||||
hasInitiatingGrant := false
|
||||
for _, grantType := range grantTypes {
|
||||
switch grantType {
|
||||
case string(fosite.GrantTypeAuthorizationCode), string(fosite.GrantTypeDeviceCode):
|
||||
hasInitiatingGrant = true
|
||||
case string(fosite.GrantTypeRefreshToken):
|
||||
default:
|
||||
return fmt.Errorf("client metadata document contains unsupported grant_type %q", grantType)
|
||||
}
|
||||
}
|
||||
if !hasInitiatingGrant {
|
||||
return errors.New("client metadata document must enable authorization_code or device_code")
|
||||
// Require a flow that can initiate authorization (ignoring the grant types Pocket ID does not implement)
|
||||
if !hasInitiatingGrantType(supportedMetadataGrantTypes(doc.GrantTypes)) {
|
||||
return errInitiatingGrantRequired
|
||||
}
|
||||
|
||||
// Pocket ID only implements the code response type for metadata clients
|
||||
@@ -162,6 +149,30 @@ func (cimdPolicy) ValidateCIMDClient(_ context.Context, doc *fosite.ClientMetada
|
||||
return nil
|
||||
}
|
||||
|
||||
var errInitiatingGrantRequired = errors.New("client metadata document must enable authorization_code or device_code")
|
||||
|
||||
// supportedMetadataGrantTypes keeps only the grant types Pocket ID implements for metadata clients and drops the rest
|
||||
func supportedMetadataGrantTypes(grantTypes []string) []string {
|
||||
if len(grantTypes) == 0 {
|
||||
// Per RFC 7591 section 2: an omitted grant_types defaults to authorization_code
|
||||
return []string{string(fosite.GrantTypeAuthorizationCode)}
|
||||
}
|
||||
|
||||
supported := make([]string, 0, len(grantTypes))
|
||||
for _, grantType := range grantTypes {
|
||||
switch grantType {
|
||||
case string(fosite.GrantTypeAuthorizationCode), string(fosite.GrantTypeDeviceCode), string(fosite.GrantTypeRefreshToken):
|
||||
supported = append(supported, grantType)
|
||||
}
|
||||
}
|
||||
return supported
|
||||
}
|
||||
|
||||
func hasInitiatingGrantType(grantTypes []string) bool {
|
||||
return slices.Contains(grantTypes, string(fosite.GrantTypeAuthorizationCode)) ||
|
||||
slices.Contains(grantTypes, string(fosite.GrantTypeDeviceCode))
|
||||
}
|
||||
|
||||
// validateMetadataRedirectURIs rejects self-asserted redirect URIs Pocket ID must not accept
|
||||
func validateMetadataRedirectURIs(field string, uris []string) error {
|
||||
for _, raw := range uris {
|
||||
@@ -193,11 +204,10 @@ func buildClientFromMetadata(doc *fosite.ClientMetadataDocument, rawURL string)
|
||||
return model.OidcClient{}, err
|
||||
}
|
||||
|
||||
// Record what the document says the client restricts itself to, so it is not silently granted capabilities it never declared
|
||||
// RFC 7591 section 2 defaults an omitted grant_types to authorization_code
|
||||
grantTypes := doc.GrantTypes
|
||||
if len(grantTypes) == 0 {
|
||||
grantTypes = []string{"authorization_code"}
|
||||
// Record the supported grant types the document declares, so the client is neither silently granted capabilities it never declared nor persisted with grants Pocket ID cannot honor
|
||||
grantTypes := supportedMetadataGrantTypes(doc.GrantTypes)
|
||||
if !hasInitiatingGrantType(grantTypes) {
|
||||
return model.OidcClient{}, errInitiatingGrantRequired
|
||||
}
|
||||
|
||||
client := model.OidcClient{
|
||||
|
||||
@@ -52,6 +52,29 @@ func TestBuildClientFromMetadata(t *testing.T) {
|
||||
}
|
||||
})
|
||||
|
||||
t.Run("unsupported grant types are dropped", func(t *testing.T) {
|
||||
doc := &fosite.ClientMetadataDocument{
|
||||
ClientID: id,
|
||||
RedirectURIs: []string{"https://app.example.com/callback"},
|
||||
TokenEndpointAuthMethod: "none",
|
||||
GrantTypes: []string{"authorization_code", "refresh_token", "urn:ietf:params:oauth:grant-type:jwt-bearer"},
|
||||
}
|
||||
c, err := buildClientFromMetadata(doc, id)
|
||||
require.NoError(t, err)
|
||||
assert.Equal(t, []string{"authorization_code", "refresh_token"}, []string(c.MetadataGrantTypes))
|
||||
})
|
||||
|
||||
t.Run("documents without a supported initiating grant are rejected", func(t *testing.T) {
|
||||
doc := &fosite.ClientMetadataDocument{
|
||||
ClientID: id,
|
||||
RedirectURIs: []string{"https://app.example.com/callback"},
|
||||
TokenEndpointAuthMethod: "none",
|
||||
GrantTypes: []string{"refresh_token", "urn:ietf:params:oauth:grant-type:jwt-bearer"},
|
||||
}
|
||||
_, err := buildClientFromMetadata(doc, id)
|
||||
require.ErrorIs(t, err, errInitiatingGrantRequired)
|
||||
})
|
||||
|
||||
t.Run("name falls back to the client ID host", func(t *testing.T) {
|
||||
c, err := buildClientFromMetadata(&fosite.ClientMetadataDocument{ClientID: id, TokenEndpointAuthMethod: "none"}, id)
|
||||
require.NoError(t, err)
|
||||
@@ -71,7 +94,8 @@ func TestCIMDPolicyValidate(t *testing.T) {
|
||||
{name: "defaults are supported"},
|
||||
{name: "authorization code and refresh token are supported", grantTypes: []string{"authorization_code", "refresh_token"}},
|
||||
{name: "device code is supported", grantTypes: []string{string(fosite.GrantTypeDeviceCode)}},
|
||||
{name: "client credentials is rejected", grantTypes: []string{"client_credentials"}, wantError: "unsupported grant_type"},
|
||||
{name: "unsupported grant types are ignored", grantTypes: []string{"authorization_code", "refresh_token", "urn:ietf:params:oauth:grant-type:jwt-bearer"}},
|
||||
{name: "client credentials cannot initiate authorization", grantTypes: []string{"client_credentials"}, wantError: "must enable"},
|
||||
{name: "refresh token cannot initiate authorization", grantTypes: []string{"refresh_token"}, wantError: "must enable"},
|
||||
{name: "implicit response is rejected", grantTypes: []string{"authorization_code"}, responseTypes: []string{"token"}, wantError: "unsupported response_type"},
|
||||
} {
|
||||
|
||||
@@ -212,6 +212,7 @@ func (s *TestService) SeedDatabase(baseURL string) error {
|
||||
ID: "606c7782-f2b1-49e5-8ea9-26eb1b06d018",
|
||||
},
|
||||
Name: "Immich",
|
||||
LaunchURL: new("https://immich.local"),
|
||||
Credentials: seededClientCredentials("2f1b8f1a-1d3e-4f0c-9c1a-000000000002", "PYjrE9u4v9GVqXKi52eur0eb2Ci4kc0x"),
|
||||
CallbackURLs: datatype.StringList{"http://immich.localhost/auth/callback"},
|
||||
CreatedByID: new(users[1].ID),
|
||||
|
||||
@@ -668,6 +668,16 @@ func (s *OidcService) ListAuthorizedClients(ctx context.Context, userID string,
|
||||
Preload("Client").
|
||||
Where("user_id = ?", userID)
|
||||
|
||||
// Apply the launch URL filter before pagination so hidden authorizations have their own page count
|
||||
if hasLaunchURL, ok := getHasLaunchURLFilter(listRequestOptions); ok {
|
||||
query = query.Joins("JOIN oidc_clients ON oidc_clients.id = user_authorized_oidc_clients.client_id")
|
||||
if hasLaunchURL {
|
||||
query = query.Where("oidc_clients.launch_url IS NOT NULL AND oidc_clients.launch_url <> ''")
|
||||
} else {
|
||||
query = query.Where("oidc_clients.launch_url IS NULL OR oidc_clients.launch_url = ''")
|
||||
}
|
||||
}
|
||||
|
||||
var authorizedClients []model.UserAuthorizedOidcClient
|
||||
response, err := utils.PaginateFilterAndSort(listRequestOptions, query, &authorizedClients)
|
||||
|
||||
@@ -743,6 +753,15 @@ func (s *OidcService) ListAccessibleOidcClients(ctx context.Context, userID stri
|
||||
WHERE oidc_clients_allowed_user_groups.oidc_client_id = oidc_clients.id
|
||||
AND oidc_clients_allowed_user_groups.user_group_id IN (?))`, false, userGroupIDs)
|
||||
|
||||
// Apply the launch URL filter before pagination so the app launcher never contains empty pages
|
||||
if hasLaunchURL, ok := getHasLaunchURLFilter(listRequestOptions); ok {
|
||||
if hasLaunchURL {
|
||||
query = query.Where("oidc_clients.launch_url IS NOT NULL AND oidc_clients.launch_url <> ''")
|
||||
} else {
|
||||
query = query.Where("oidc_clients.launch_url IS NULL OR oidc_clients.launch_url = ''")
|
||||
}
|
||||
}
|
||||
|
||||
var clients []model.OidcClient
|
||||
|
||||
// Handle custom sorting for lastUsedAt column
|
||||
@@ -781,6 +800,16 @@ func (s *OidcService) ListAccessibleOidcClients(ctx context.Context, userID stri
|
||||
return dtos, response, err
|
||||
}
|
||||
|
||||
func getHasLaunchURLFilter(listRequestOptions utils.ListRequestOptions) (bool, bool) {
|
||||
values := listRequestOptions.Filters["hasLaunchURL"]
|
||||
if len(values) == 0 {
|
||||
return false, false
|
||||
}
|
||||
|
||||
hasLaunchURL, ok := values[0].(bool)
|
||||
return hasLaunchURL, ok
|
||||
}
|
||||
|
||||
func (s *OidcService) GetClientPreview(ctx context.Context, clientID string, userID string, scopes []string, authenticationMethod string) (*dto.OidcClientPreviewDto, error) {
|
||||
client, err := s.getClientInternal(ctx, clientID, s.db, false)
|
||||
if err != nil {
|
||||
|
||||
@@ -971,6 +971,60 @@ func TestOidcService_ListAccessibleOidcClients_requiresExplicitGroupPermission(t
|
||||
assert.Equal(t, []string{"Unrestricted"}, accessibleClientNames(noGroupClients))
|
||||
}
|
||||
|
||||
func TestOidcService_ListClientViewsFilterByLaunchURLPresence(t *testing.T) {
|
||||
db := testutils.NewDatabaseForTest(t)
|
||||
s, err := NewOidcService(db, nil, nil, nil, nil, nil, nil)
|
||||
require.NoError(t, err)
|
||||
|
||||
user := model.User{Username: "launch-url-filter"}
|
||||
require.NoError(t, db.Create(&user).Error)
|
||||
|
||||
launchURL := "https://launchable.example.com"
|
||||
emptyLaunchURL := ""
|
||||
clients := []model.OidcClient{
|
||||
{Name: "Launchable", LaunchURL: &launchURL},
|
||||
{Name: "Missing launch URL"},
|
||||
{Name: "Empty launch URL", LaunchURL: &emptyLaunchURL},
|
||||
}
|
||||
for i := range clients {
|
||||
require.NoError(t, db.Create(&clients[i]).Error)
|
||||
require.NoError(t, db.Create(&model.UserAuthorizedOidcClient{
|
||||
UserID: user.ID,
|
||||
ClientID: clients[i].ID,
|
||||
}).Error)
|
||||
}
|
||||
|
||||
withLaunchURL := utils.ListRequestOptions{
|
||||
Filters: map[string][]any{"hasLaunchURL": {true}},
|
||||
}
|
||||
withoutLaunchURL := utils.ListRequestOptions{
|
||||
Filters: map[string][]any{"hasLaunchURL": {false}},
|
||||
}
|
||||
|
||||
allClients, allClientsPagination, err := s.ListAccessibleOidcClients(t.Context(), user.ID, utils.ListRequestOptions{})
|
||||
require.NoError(t, err)
|
||||
assert.Equal(t, int64(3), allClientsPagination.TotalItems)
|
||||
assert.ElementsMatch(t, []string{"Launchable", "Missing launch URL", "Empty launch URL"}, accessibleClientNames(allClients))
|
||||
|
||||
launchableClients, launchablePagination, err := s.ListAccessibleOidcClients(t.Context(), user.ID, withLaunchURL)
|
||||
require.NoError(t, err)
|
||||
assert.Equal(t, int64(1), launchablePagination.TotalItems)
|
||||
assert.Equal(t, []string{"Launchable"}, accessibleClientNames(launchableClients))
|
||||
|
||||
allAuthorizations, allAuthorizationsPagination, err := s.ListAuthorizedClients(t.Context(), user.ID, utils.ListRequestOptions{})
|
||||
require.NoError(t, err)
|
||||
assert.Equal(t, int64(3), allAuthorizationsPagination.TotalItems)
|
||||
assert.Len(t, allAuthorizations, 3)
|
||||
|
||||
hiddenAuthorizations, hiddenPagination, err := s.ListAuthorizedClients(t.Context(), user.ID, withoutLaunchURL)
|
||||
require.NoError(t, err)
|
||||
assert.Equal(t, int64(2), hiddenPagination.TotalItems)
|
||||
assert.ElementsMatch(t, []string{"Missing launch URL", "Empty launch URL"}, []string{
|
||||
hiddenAuthorizations[0].Client.Name,
|
||||
hiddenAuthorizations[1].Client.Name,
|
||||
})
|
||||
}
|
||||
|
||||
func accessibleClientNames(clients []dto.AccessibleOidcClientDto) []string {
|
||||
names := make([]string, len(clients))
|
||||
for i := range clients {
|
||||
|
||||
File diff suppressed because one or more lines are too long
Reference in New Issue
Block a user