diff --git a/internal/auth/auth.go b/internal/auth/auth.go index 4ce2966..102619b 100644 --- a/internal/auth/auth.go +++ b/internal/auth/auth.go @@ -23,41 +23,6 @@ import ( "golang.org/x/oauth2" ) -var oauthScope = strings.Join([]string{ - "openid", - "profile", - "email", - "offline_access", - "me:read", - "me:write", - "todo:read", - "todo:write", - "homework:read", - "homework:write", - "subscription:read", - "subscription:write", - "comment:read", - "comment:write", - "description:read", - "description:write", - "upload:read", - "upload:write", - "dashboard:read", - "dashboard:write", - "bus:read", - "bus:write", - "course:read", - "course:write", - "section:read", - "section:write", - "teacher:read", - "teacher:write", - "schedule:read", - "schedule:write", - "exam:read", - "exam:write", -}, " ") - func b64url(data []byte) string { return base64.RawURLEncoding.EncodeToString(data) } @@ -123,13 +88,39 @@ func oauthResource(server string, meta map[string]any) string { return strings.TrimRight(server, "/") } -func registerPublicClient(endpoint string, redirectURIs, grantTypes, responseTypes []string) (map[string]any, error) { +func oauthScopesFromMetadata(meta map[string]any) ([]string, error) { + rawScopes, ok := meta["scopes_supported"].([]any) + if !ok || len(rawScopes) == 0 { + return nil, fmt.Errorf("server OAuth metadata does not advertise scopes_supported") + } + + scopes := make([]string, 0, len(rawScopes)) + seen := make(map[string]struct{}, len(rawScopes)) + for _, rawScope := range rawScopes { + scope, ok := rawScope.(string) + if !ok || strings.TrimSpace(scope) == "" { + return nil, fmt.Errorf("server OAuth metadata contains an invalid supported scope") + } + scope = strings.TrimSpace(scope) + if _, duplicate := seen[scope]; duplicate { + continue + } + seen[scope] = struct{}{} + scopes = append(scopes, scope) + } + if _, ok := seen["openid"]; !ok { + return nil, fmt.Errorf("server OAuth metadata does not support the required openid scope") + } + return scopes, nil +} + +func registerPublicClient(endpoint string, scopes, redirectURIs, grantTypes, responseTypes []string) (map[string]any, error) { body := map[string]any{ "client_name": "life-ustc-cli", "application_type": "native", "token_endpoint_auth_method": "none", "grant_types": grantTypes, - "scope": oauthScope, + "scope": strings.Join(scopes, " "), } if len(redirectURIs) > 0 { body["redirect_uris"] = redirectURIs @@ -155,9 +146,10 @@ func registerPublicClient(endpoint string, redirectURIs, grantTypes, responseTyp return result, nil } -func registerClient(endpoint, redirectURI string) (map[string]any, error) { +func registerClient(endpoint, redirectURI string, scopes []string) (map[string]any, error) { return registerPublicClient( endpoint, + scopes, []string{redirectURI}, []string{"authorization_code", "refresh_token"}, []string{"code"}, @@ -307,6 +299,11 @@ func Login(server string) (*config.Credential, error) { return nil, fmt.Errorf("server does not advertise a registration_endpoint") } resource := oauthResource(server, meta) + scopes, err := oauthScopesFromMetadata(meta) + if err != nil { + return nil, err + } + scope := strings.Join(scopes, " ") // Start local callback server listener, err := net.Listen("tcp", "127.0.0.1:0") @@ -317,7 +314,7 @@ func Login(server string) (*config.Credential, error) { redirectURI := callbackRedirectURI(listener.Addr()) // Register client - clientInfo, err := registerClient(regEndpoint, redirectURI) + clientInfo, err := registerClient(regEndpoint, redirectURI, scopes) if err != nil { return nil, err } @@ -336,7 +333,7 @@ func Login(server string) (*config.Credential, error) { conf := &oauth2.Config{ ClientID: clientID, RedirectURL: redirectURI, - Scopes: strings.Fields(oauthScope), + Scopes: scopes, Endpoint: oauth2.Endpoint{ AuthURL: authEndpoint, TokenURL: tokenEndpoint, @@ -395,7 +392,7 @@ func Login(server string) (*config.Credential, error) { } vt := newVerifiedToken(tok) - if err := requireIDTokenForOpenID(oauthScope, vt.IDToken); err != nil { + if err := requireIDTokenForOpenID(effectiveTokenScope(vt, scope), vt.IDToken); err != nil { return nil, err } issuer := stringFromMap(meta, "issuer") @@ -405,7 +402,7 @@ func Login(server string) (*config.Credential, error) { if err := vt.ValidateIDToken(issuer, clientID); err != nil { return nil, err } - return verifiedTokenToCredential(clientID, resource, vt, "", oauthScope, time.Now()) + return verifiedTokenToCredential(clientID, resource, vt, "", scope, time.Now()) } // RefreshToken attempts to refresh the access token. @@ -431,9 +428,6 @@ func RefreshToken(server string, cred *config.Credential) (*config.Credential, e } vt := newVerifiedToken(tok) - if err := requireIDTokenForOpenID(oauthScope, vt.IDToken); err != nil { - return nil, err - } issuer := stringFromMap(meta, "issuer") if issuer == "" { issuer = server diff --git a/internal/auth/auth_test.go b/internal/auth/auth_test.go index 641547d..05b6035 100644 --- a/internal/auth/auth_test.go +++ b/internal/auth/auth_test.go @@ -12,6 +12,7 @@ import ( "testing" "time" + "github.com/Life-USTC/CLI/internal/config" "github.com/go-jose/go-jose/v4" "github.com/go-jose/go-jose/v4/jwt" ) @@ -33,6 +34,7 @@ func TestRegisterPublicClientUsesNativeApplicationType(t *testing.T) { _, err := registerPublicClient( server.URL, + []string{"openid", "profile", "workspace.todo:read"}, []string{"http://127.0.0.1:46289/callback"}, []string{"authorization_code", "refresh_token"}, []string{"code"}, @@ -44,6 +46,9 @@ func TestRegisterPublicClientUsesNativeApplicationType(t *testing.T) { if body["application_type"] != "native" { t.Fatalf("application_type = %#v, want native", body["application_type"]) } + if body["scope"] != "openid profile workspace.todo:read" { + t.Fatalf("scope = %#v", body["scope"]) + } redirectURIs, ok := body["redirect_uris"].([]any) if !ok || len(redirectURIs) != 1 || redirectURIs[0] != "http://127.0.0.1:46289/callback" { t.Fatalf("redirect_uris = %#v", body["redirect_uris"]) @@ -66,6 +71,7 @@ func TestRegisterPublicClientOmitsUnusedDeviceRedirectMetadata(t *testing.T) { _, err := registerPublicClient( server.URL, + []string{"openid", "profile", "workspace.todo:read"}, nil, []string{"urn:ietf:params:oauth:grant-type:device_code", "refresh_token"}, nil, @@ -85,6 +91,56 @@ func TestRegisterPublicClientOmitsUnusedDeviceRedirectMetadata(t *testing.T) { } } +func TestOAuthScopesFromMetadata(t *testing.T) { + scopes, err := oauthScopesFromMetadata(map[string]any{ + "scopes_supported": []any{ + "openid", + "profile", + "workspace.todo:read", + "workspace.todo:write", + "workspace.todo:read", + }, + }) + if err != nil { + t.Fatal(err) + } + want := []string{ + "openid", + "profile", + "workspace.todo:read", + "workspace.todo:write", + } + if len(scopes) != len(want) { + t.Fatalf("scopes = %#v, want %#v", scopes, want) + } + for i := range want { + if scopes[i] != want[i] { + t.Fatalf("scopes = %#v, want %#v", scopes, want) + } + } +} + +func TestOAuthScopesFromMetadataRejectsMissingOrInvalidScopes(t *testing.T) { + tests := []struct { + name string + meta map[string]any + }{ + {name: "missing", meta: map[string]any{}}, + {name: "wrong type", meta: map[string]any{"scopes_supported": "openid profile"}}, + {name: "empty", meta: map[string]any{"scopes_supported": []any{}}}, + {name: "invalid item", meta: map[string]any{"scopes_supported": []any{"openid", 42}}}, + {name: "blank item", meta: map[string]any{"scopes_supported": []any{"openid", " "}}}, + {name: "missing openid", meta: map[string]any{"scopes_supported": []any{"profile"}}}, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + if _, err := oauthScopesFromMetadata(tt.meta); err == nil { + t.Fatal("expected invalid metadata error") + } + }) + } +} + func TestOAuthCallbackHandlerDeliversOnlyFirstRequest(t *testing.T) { results := make(chan callbackResult, 1) handler := oauthCallbackHandler(results) @@ -200,6 +256,62 @@ func TestRequireIDTokenForOpenID(t *testing.T) { } } +func TestEffectiveTokenScopePrefersGrantedScope(t *testing.T) { + vt := &VerifiedToken{Scope: "profile workspace.todo:read"} + if got := effectiveTokenScope(vt, "openid profile workspace.todo:read"); got != "profile workspace.todo:read" { + t.Fatalf("effective scope = %q", got) + } + if err := requireIDTokenForOpenID(effectiveTokenScope(vt, "openid profile"), ""); err != nil { + t.Fatalf("reduced grant without openid should not require an ID token: %v", err) + } +} + +func TestRefreshTokenDoesNotRequireNewIDToken(t *testing.T) { + var server *httptest.Server + server = httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + w.Header().Set("Content-Type", "application/json") + switch r.URL.Path { + case "/.well-known/oauth-authorization-server/api/auth": + _ = json.NewEncoder(w).Encode(map[string]any{ + "issuer": server.URL + "/api/auth", + "token_endpoint": server.URL + "/api/auth/oauth2/token", + }) + case "/api/auth/oauth2/token": + if err := r.ParseForm(); err != nil { + http.Error(w, err.Error(), http.StatusBadRequest) + return + } + if r.Form.Get("grant_type") != "refresh_token" { + http.Error(w, "unexpected grant", http.StatusBadRequest) + return + } + _ = json.NewEncoder(w).Encode(map[string]any{ + "access_token": "next-access", + "token_type": "Bearer", + "expires_in": 3600, + }) + default: + http.NotFound(w, r) + } + })) + t.Cleanup(server.Close) + + cred, err := RefreshToken(server.URL, &config.Credential{ + ClientID: "client-1", + RefreshToken: "refresh-1", + Scope: "openid profile workspace.todo:read", + }) + if err != nil { + t.Fatal(err) + } + if cred.AccessToken != "next-access" || cred.RefreshToken != "refresh-1" { + t.Fatalf("credential = %#v", cred) + } + if cred.Scope != "openid profile workspace.todo:read" { + t.Fatalf("scope = %q", cred.Scope) + } +} + func TestValidateIDTokenAudienceIsClientID(t *testing.T) { key, err := rsa.GenerateKey(rand.Reader, 2048) if err != nil { diff --git a/internal/auth/device.go b/internal/auth/device.go index b37edd2..8385f78 100644 --- a/internal/auth/device.go +++ b/internal/auth/device.go @@ -34,10 +34,16 @@ func LoginDeviceCode(server string) (*config.Credential, error) { return nil, fmt.Errorf("server does not advertise a registration_endpoint") } resource := oauthResource(server, meta) + scopes, err := oauthScopesFromMetadata(meta) + if err != nil { + return nil, err + } + scope := strings.Join(scopes, " ") // Register client clientInfo, err := registerPublicClient( regEndpoint, + scopes, nil, []string{"urn:ietf:params:oauth:grant-type:device_code", "refresh_token"}, nil, @@ -49,7 +55,7 @@ func LoginDeviceCode(server string) (*config.Credential, error) { conf := &oauth2.Config{ ClientID: clientID, - Scopes: strings.Fields(oauthScope), + Scopes: scopes, Endpoint: oauth2.Endpoint{ DeviceAuthURL: deviceEndpoint, TokenURL: tokenEndpoint, @@ -88,7 +94,7 @@ func LoginDeviceCode(server string) (*config.Credential, error) { } vt := newVerifiedToken(tok) - if err := requireIDTokenForOpenID(oauthScope, vt.IDToken); err != nil { + if err := requireIDTokenForOpenID(effectiveTokenScope(vt, scope), vt.IDToken); err != nil { return nil, err } issuer := stringFromMap(meta, "issuer") @@ -98,5 +104,5 @@ func LoginDeviceCode(server string) (*config.Credential, error) { if err := vt.ValidateIDToken(issuer, clientID); err != nil { return nil, err } - return verifiedTokenToCredential(clientID, resource, vt, "", oauthScope, time.Now()) + return verifiedTokenToCredential(clientID, resource, vt, "", scope, time.Now()) } diff --git a/internal/auth/oauth.go b/internal/auth/oauth.go index 6ce8eb5..c5894b3 100644 --- a/internal/auth/oauth.go +++ b/internal/auth/oauth.go @@ -143,6 +143,13 @@ func requireIDTokenForOpenID(scope, idToken string) error { return nil } +func effectiveTokenScope(token *VerifiedToken, fallback string) string { + if token != nil && strings.TrimSpace(token.Scope) != "" { + return strings.TrimSpace(token.Scope) + } + return strings.TrimSpace(fallback) +} + func verifiedTokenToCredential(clientID, resource string, vt *VerifiedToken, fallbackRefresh, fallbackScope string, now time.Time) (*config.Credential, error) { if vt == nil { return nil, errors.New("token response is nil") @@ -159,10 +166,7 @@ func verifiedTokenToCredential(clientID, resource string, vt *VerifiedToken, fal if refreshToken == "" { refreshToken = fallbackRefresh } - scope := strings.TrimSpace(vt.Scope) - if scope == "" { - scope = fallbackScope - } + scope := effectiveTokenScope(vt, fallbackScope) return &config.Credential{ ClientID: clientID, AccessToken: accessToken,