Skip to content

Fix OAuth scope discovery #37

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
tiankaima merged 1 commit into from
Aug 11, 2026
Merged

Fix OAuth scope discovery #37

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
84 changes: 39 additions & 45 deletions internal/auth/auth.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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)
}
Expand Down Expand Up @@ -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
Expand All @@ -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"},
Expand Down Expand Up @@ -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")
Expand All @@ -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
}
Expand All @@ -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,
Expand Down Expand Up @@ -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")
Expand All @@ -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.
Expand All @@ -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
Expand Down
112 changes: 112 additions & 0 deletions internal/auth/auth_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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"
)
Expand All @@ -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"},
Expand All @@ -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"])
Expand All @@ -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,
Expand All @@ -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)
Expand Down Expand Up @@ -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 {
Expand Down
12 changes: 9 additions & 3 deletions internal/auth/device.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand All @@ -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,
Expand Down Expand Up @@ -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")
Expand All @@ -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())
}
12 changes: 8 additions & 4 deletions internal/auth/oauth.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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")
Expand All @@ -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,
Expand Down