From 277abc89636bab66d74dbdf9f87a842703f3fe47 Mon Sep 17 00:00:00 2001 From: evlic Date: Wed, 12 Aug 2026 17:29:04 +0800 Subject: [PATCH] fix(auth): address OAuth review feedback --- server/internal/api/oauth_test.go | 10 +- server/internal/config/member.go | 29 +++--- server/internal/http/legacy/session.go | 4 +- server/internal/member/manager.go | 7 +- server/openapi.yaml | 104 +++++++++++++++++++ webpage/docs/configuration/authentication.md | 14 ++- 6 files changed, 142 insertions(+), 26 deletions(-) diff --git a/server/internal/api/oauth_test.go b/server/internal/api/oauth_test.go index f7f6100f..2e15a0e1 100644 --- a/server/internal/api/oauth_test.go +++ b/server/internal/api/oauth_test.go @@ -64,7 +64,13 @@ func TestOAuthLoginSynchronizesProfile(t *testing.T) { AdminEmails: []string{"admin@example.test"}, SuccessRedirect: "/room", UserProfile: types.MemberProfile{ - CanLogin: true, + CanLogin: true, + CanAccessClipboard: true, + }, + AdminProfile: types.MemberProfile{ + IsAdmin: true, + CanLogin: true, + CanSeeInactiveCursors: true, }, }, } @@ -113,7 +119,7 @@ func TestOAuthLoginSynchronizesProfile(t *testing.T) { t.Fatal("OAuth session was not created") } profile := userSession.Profile() - if profile.Name != "Ada Lovelace" || profile.Avatar != "https://example.test/ada.png" || !profile.IsAdmin { + if profile.Name != "Ada Lovelace" || profile.Avatar != "https://example.test/ada.png" || !profile.IsAdmin || profile.CanAccessClipboard { t.Fatalf("profile = %#v", profile) } if extraData := members.OAuthExtraData(userSession.ID()); extraData["displayName"] != "Ada Lovelace" || extraData["isAdmin"] != "true" { diff --git a/server/internal/config/member.go b/server/internal/config/member.go index a15a7f75..21f0a72d 100644 --- a/server/internal/config/member.go +++ b/server/internal/config/member.go @@ -1,8 +1,6 @@ package config import ( - "strings" - "github.com/rs/zerolog/log" "github.com/spf13/cobra" "github.com/spf13/viper" @@ -43,6 +41,7 @@ type OAuth struct { UsernameField string AvatarField string SuccessRedirect string + AdminProfile types.MemberProfile UserProfile types.MemberProfile } @@ -106,8 +105,8 @@ func (Member) Init(cmd *cobra.Command) error { return err } - cmd.PersistentFlags().String("member.oauth.admin_email", "", "comma-separated OAuth user-info email addresses granted administrator access") - if err := viper.BindPFlag("member.oauth.admin_email", cmd.PersistentFlags().Lookup("member.oauth.admin_email")); err != nil { + cmd.PersistentFlags().StringSlice("member.oauth.admin_emails", []string{}, "OAuth user-info email addresses granted administrator access") + if err := viper.BindPFlag("member.oauth.admin_emails", cmd.PersistentFlags().Lookup("member.oauth.admin_emails")); err != nil { return err } @@ -176,6 +175,11 @@ func (Member) Init(cmd *cobra.Command) error { return err } + cmd.PersistentFlags().String("member.oauth.admin_profile", "{}", "OAuth administrator permission profile") + if err := viper.BindPFlag("member.oauth.admin_profile", cmd.PersistentFlags().Lookup("member.oauth.admin_profile")); err != nil { + return err + } + return nil } @@ -215,7 +219,7 @@ func (s *Member) Set() { s.OAuth.Enabled = viper.GetBool("member.oauth.enabled") s.OAuth.AutoRedirect = viper.GetBool("member.oauth.auto_redirect") s.OAuth.Name = viper.GetString("member.oauth.name") - s.OAuth.AdminEmails = commaSeparatedValues(viper.GetString("member.oauth.admin_email")) + s.OAuth.AdminEmails = viper.GetStringSlice("member.oauth.admin_emails") s.OAuth.ClientID = viper.GetString("member.oauth.client_id") s.OAuth.ClientSecret = viper.GetString("member.oauth.client_secret") s.OAuth.IssuerURL = viper.GetString("member.oauth.issuer_url") @@ -274,17 +278,12 @@ func (s *Member) Set() { )); err != nil { log.Warn().Err(err).Msgf("unable to parse member multiuser admin profile") } -} - -func commaSeparatedValues(value string) []string { - values := strings.Split(value, ",") - result := make([]string, 0, len(values)) - for _, value := range values { - if value = strings.TrimSpace(value); value != "" { - result = append(result, value) - } + s.OAuth.AdminProfile = s.Multiuser.AdminProfile + if err := viper.UnmarshalKey("member.oauth.admin_profile", &s.OAuth.AdminProfile, viper.DecodeHook( + utils.JsonStringAutoDecode(s.OAuth.AdminProfile), + )); err != nil { + log.Warn().Err(err).Msgf("unable to parse member OAuth admin profile") } - return result } func (s *Member) SetV2() { diff --git a/server/internal/http/legacy/session.go b/server/internal/http/legacy/session.go index 19e648fc..ce9d135c 100644 --- a/server/internal/http/legacy/session.go +++ b/server/internal/http/legacy/session.go @@ -203,10 +203,8 @@ func (s *session) create(username, password string) error { s.setSessionData(data) - // Cookie-based authentication is supported when the browser presents an - // existing session above. Password login still requires an API token. if s.token == "" { - return fmt.Errorf("token not found - make sure you are not using Cookie auth on the server") + return fmt.Errorf("token not found") } return nil diff --git a/server/internal/member/manager.go b/server/internal/member/manager.go index 10c4fdc8..c40c2086 100644 --- a/server/internal/member/manager.go +++ b/server/internal/member/manager.go @@ -182,14 +182,13 @@ func (manager *MemberManagerCtx) LoginOAuth(subject, name, avatar, email string, } profile := manager.config.OAuth.UserProfile + if isOAuthAdministrator(email, isAdmin, manager.config.OAuth.AdminEmails) { + profile = manager.config.OAuth.AdminProfile + } if name != "" { profile.Name = name } profile.Avatar = avatar - if isOAuthAdministrator(email, isAdmin, manager.config.OAuth.AdminEmails) { - profile.IsAdmin = true - profile.CanSeeInactiveCursors = true - } if !profile.IsAdmin && manager.sessions.Settings().LockedLogins { return nil, "", types.ErrSessionLoginsLocked diff --git a/server/openapi.yaml b/server/openapi.yaml index ce8e44dc..054cc3f2 100644 --- a/server/openapi.yaml +++ b/server/openapi.yaml @@ -168,6 +168,94 @@ paths: schema: $ref: '#/components/schemas/SessionLoginRequest' required: true + /api/oauth/config: + get: + tags: + - current-session + summary: Get OAuth Login Configuration + description: Retrieve the OAuth login option shown by the client. This endpoint does not expose OAuth client credentials. + operationId: oauthConfig + security: [] + responses: + '200': + description: OAuth login configuration retrieved successfully. + content: + application/json: + schema: + $ref: '#/components/schemas/OAuthLoginConfig' + /api/oauth/login: + get: + tags: + - current-session + summary: Start OAuth Login + description: Start the OAuth authorization-code flow and redirect to the configured identity provider. + operationId: oauthLogin + security: [] + responses: + '302': + description: Redirect to the identity provider authorization endpoint. + '404': + $ref: '#/components/responses/NotFound' + '503': + description: OAuth is not configured, issuer discovery failed, or session cookies are disabled. + content: + application/json: + schema: + $ref: '#/components/schemas/ErrorMessage' + /api/oauth/callback: + get: + tags: + - current-session + summary: Complete OAuth Login + description: Complete the OAuth authorization-code flow, create a session cookie, and redirect to the configured success URL. + operationId: oauthCallback + security: [] + parameters: + - in: query + name: state + description: State value returned by the identity provider. + required: false + schema: + type: string + - in: query + name: code + description: Authorization code returned by the identity provider. + required: false + schema: + type: string + - in: query + name: error + description: Authorization error returned by the identity provider. + required: false + schema: + type: string + responses: + '303': + description: OAuth login succeeded and the response sets the session cookie before redirecting. + '400': + description: OAuth callback parameters or state are invalid. + content: + application/json: + schema: + $ref: '#/components/schemas/ErrorMessage' + '401': + description: OAuth authorization, token exchange, or user-info request failed. + content: + application/json: + schema: + $ref: '#/components/schemas/ErrorMessage' + '422': + description: The OAuth user already has a connected session. + content: + application/json: + schema: + $ref: '#/components/schemas/ErrorMessage' + '503': + description: OAuth is not fully configured or issuer discovery failed. + content: + application/json: + schema: + $ref: '#/components/schemas/ErrorMessage' /api/logout: post: tags: @@ -1302,6 +1390,22 @@ components: type: string description: The session token, only if cookie authentication is disabled. + OAuthLoginConfig: + type: object + properties: + enabled: + type: boolean + description: Indicates whether OAuth login is enabled for the selected member provider. + name: + type: string + description: Display name for the OAuth login option. + login_url: + type: string + description: Relative URL that starts the OAuth login flow. + password_login_enabled: + type: boolean + description: Indicates whether password login remains available. + SessionData: type: object properties: diff --git a/webpage/docs/configuration/authentication.md b/webpage/docs/configuration/authentication.md index c499a823..d0db2813 100644 --- a/webpage/docs/configuration/authentication.md +++ b/webpage/docs/configuration/authentication.md @@ -191,7 +191,7 @@ member: # When true, visiting the Neko root page immediately starts OAuth login. auto_redirect: true name: "Example SSO" - admin_email: "platform-admin@example.com,security@example.com" + admin_emails: ["platform-admin@example.com", "security@example.com"] client_id: "" client_secret: "" issuer_url: "https://id.example.com" @@ -211,11 +211,21 @@ member: can_access_clipboard: true sends_inactive_cursor: true can_see_inactive_cursors: false + admin_profile: + is_admin: true + can_login: true + can_connect: true + can_watch: true + can_host: true + can_share_media: true + can_access_clipboard: true + sends_inactive_cursor: true + can_see_inactive_cursors: true ``` The sign-in endpoint is `GET /api/oauth/login`; the callback endpoint is `GET /api/oauth/callback`. OAuth login requires `session.cookie.enabled: true`, which is the default. Set `member.provider: oauth` to make OAuth the only member provider. -`admin_email` is a comma-separated list matched against the standard `email` field, case-insensitively. An OAuth user is an administrator when either their email matches that list or their user-info/ID-token claims include `isAdmin: true`. The configured name and avatar fields are read from userinfo first, then fall back to the ID token when userinfo omits them. Avatar URLs are rendered by clients that support member avatars. +`admin_emails` is a list matched against the standard `email` field, case-insensitively. An OAuth user is an administrator when either their email matches that list or their user-info/ID-token claims include `isAdmin: true`; their permissions then use `admin_profile`. The configured name and avatar fields are read from userinfo first, then fall back to the ID token when userinfo omits them. Avatar URLs are rendered by clients that support member avatars. For OAuth sessions, `GET /api/whoami` includes `extra_data`: the merged userinfo and ID-token claims, with userinfo values taking precedence. It is returned only to the authenticated session owner and is intended for profile-field troubleshooting. Do not place secrets in provider profile claims.