Copilot commented on code in PR #1542:
URL: https://github.com/apache/dubbo-admin/pull/1542#discussion_r3921739305
##########
pkg/config/console/auth/config.go:
##########
@@ -19,26 +19,136 @@ package auth
import (
"errors"
+ "fmt"
+ "net/url"
+ "regexp"
+ "slices"
"github.com/apache/dubbo-admin/pkg/config"
)
-const DefaultExpirationTime = 7200
+const (
+ DefaultExpirationTime = 7200
+ DefaultSessionSecret = "secret"
+
+ MethodPassword = "password"
+ ProviderTypeGitHub = "github"
+ ProviderTypeOIDC = "oidc"
+)
+
+var providerIDPattern = regexp.MustCompile(`^[A-Za-z0-9][A-Za-z0-9._-]{0,63}$`)
+
+type ProviderConfig struct {
+ Type string `json:"type" yaml:"type"`
+ DisplayName string `json:"displayName" yaml:"displayName"`
+ Issuer string `json:"issuer,omitempty"
yaml:"issuer,omitempty"`
+ ClientID string `json:"clientId" yaml:"clientId"`
+ ClientSecret string `json:"clientSecret" yaml:"clientSecret"`
+ RedirectURL string `json:"redirectUrl" yaml:"redirectUrl"`
+ PostLoginRedirectURL string `json:"postLoginRedirectUrl"
yaml:"postLoginRedirectUrl"`
+ Scopes []string `json:"scopes,omitempty"
yaml:"scopes,omitempty"`
+}
// Config AuthConfig configure the valid user and password
type Config struct {
config.BaseConfig
- User string `json:"user"`
- Password string `json:"password"`
- ExpirationTime int `json:"expirationTime"`
+ Methods []string `json:"methods"
yaml:"methods"`
+ User string `json:"user" yaml:"user"`
+ Password string `json:"password"
yaml:"password"`
+ ExpirationTime int `json:"expirationTime"
yaml:"expirationTime"`
+ SessionSecret string `json:"sessionSecret"
yaml:"sessionSecret"`
+ SessionCookieSecure bool
`json:"sessionCookieSecure" yaml:"sessionCookieSecure"`
+ Providers map[string]ProviderConfig
`json:"providers,omitempty" yaml:"providers,omitempty"`
+}
+
+func (c *Config) Sanitize() {
+ c.Password = config.SanitizedValue
+ c.SessionSecret = config.SanitizedValue
+ for id, provider := range c.Providers {
+ provider.ClientSecret = config.SanitizedValue
+ c.Providers[id] = provider
+ }
}
func (c *Config) Validate() error {
- if c.User == "" || c.Password == "" {
+ if len(c.Methods) == 0 {
+ c.Methods = []string{MethodPassword}
+ }
+ // Methods contains built-in login methods only; OAuth and OIDC are
configured through Providers.
+ for _, method := range c.Methods {
+ if method != MethodPassword {
+ return fmt.Errorf("auth: unsupported method %q", method)
+ }
+ }
+ if slices.Contains(c.Methods, MethodPassword) && (c.User == "" ||
c.Password == "") {
return errors.New("auth: user or password is needed, but found
empty")
}
if c.ExpirationTime <= 0 || c.ExpirationTime >= 24*60*60 {
return errors.New("auth: expirationTime should be greater than
0 and less than 86400")
}
+ if c.SessionSecret == "" {
+ c.SessionSecret = DefaultSessionSecret
+ }
+ for id, provider := range c.Providers {
+ if err := validateProvider(id, &provider); err != nil {
+ return err
+ }
+ c.Providers[id] = provider
+ }
return nil
}
+
+func validateProvider(id string, provider *ProviderConfig) error {
+ if !providerIDPattern.MatchString(id) {
+ return fmt.Errorf("auth: invalid provider id %q", id)
+ }
+ if provider.Type != ProviderTypeGitHub && provider.Type !=
ProviderTypeOIDC {
+ return fmt.Errorf("auth provider %q: unsupported type %q", id,
provider.Type)
+ }
+ if provider.DisplayName == "" {
+ provider.DisplayName = id
+ }
+ if provider.ClientID == "" || provider.ClientSecret == "" {
+ return fmt.Errorf("auth provider %q: clientId and clientSecret
are required", id)
+ }
+ redirect, err := validateHTTPURL(provider.RedirectURL)
+ if err != nil {
+ return fmt.Errorf("auth provider %q: invalid redirectUrl: %w",
id, err)
+ }
+ expectedPath := "/api/v1/auth/providers/" + id + "/callback"
+ // The provider must return to the callback route registered for this
provider ID.
+ if redirect.Path != expectedPath {
+ return fmt.Errorf("auth provider %q: redirectUrl must use
callback path %q", id, expectedPath)
+ }
+ if _, err := validateHTTPURL(provider.PostLoginRedirectURL); err != nil
{
+ return fmt.Errorf("auth provider %q: invalid
postLoginRedirectUrl: %w", id, err)
+ }
+ switch provider.Type {
+ case ProviderTypeGitHub:
+ if len(provider.Scopes) == 0 {
+ provider.Scopes = []string{"read:user", "user:email"}
+ }
+ case ProviderTypeOIDC:
+ if _, err := validateHTTPURL(provider.Issuer); err != nil {
+ return fmt.Errorf("auth provider %q: invalid issuer:
%w", id, err)
+ }
Review Comment:
OIDC configuration currently accepts a plaintext HTTP issuer, and discovery
only checks that token/JWKS/UserInfo endpoints are non-empty. This can send the
authorization code, client secret, access token, or signing keys over
unauthenticated HTTP. Require HTTPS for the issuer and discovered endpoints
(with any loopback development exception made explicit).
##########
pkg/console/auth/oidc.go:
##########
@@ -0,0 +1,229 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package auth
+
+import (
+ "context"
+ "crypto/subtle"
+ "encoding/json"
+ "errors"
+ "fmt"
+ "net/http"
+ "strings"
+ "time"
+
+ configauth "github.com/apache/dubbo-admin/pkg/config/console/auth"
+ jose "github.com/go-jose/go-jose/v4"
+ josejwt "github.com/go-jose/go-jose/v4/jwt"
+ "golang.org/x/oauth2"
+)
+
+type oidcDiscovery struct {
+ Issuer string `json:"issuer"`
+ AuthorizationEndpoint string `json:"authorization_endpoint"`
+ TokenEndpoint string `json:"token_endpoint"`
+ JWKSURI string `json:"jwks_uri"`
+ UserInfoEndpoint string `json:"userinfo_endpoint"`
+}
+
+type oidcProfile struct {
+ Nonce string `json:"nonce"`
+ PreferredUsername string `json:"preferred_username"`
+ Name string `json:"name"`
+ Email string `json:"email"`
+ Groups []string `json:"groups"`
+ Roles []string `json:"roles"`
+}
+
+type oidcProvider struct {
+ id string
+ displayName string
+ issuer string
+ clientID string
+ postLoginRedirectURL string
+ discovery oidcDiscovery
+ oauth oauth2.Config
+ httpClient *http.Client
+}
+
+func NewOIDCProvider(ctx context.Context, id string, cfg
configauth.ProviderConfig, client *http.Client) (Provider, error) {
+ if client == nil {
+ client = http.DefaultClient
+ }
+ discoveryURL := strings.TrimRight(cfg.Issuer, "/") +
"/.well-known/openid-configuration"
+ var discovery oidcDiscovery
+ if err := getOIDCJSON(ctx, client, discoveryURL, "", &discovery); err
!= nil {
+ return nil, fmt.Errorf("discover OIDC provider %q: %w", id, err)
+ }
+ if discovery.Issuer != cfg.Issuer {
+ return nil, fmt.Errorf("OIDC provider %q discovery issuer %q
does not match configured issuer %q", id, discovery.Issuer, cfg.Issuer)
+ }
+ if discovery.AuthorizationEndpoint == "" || discovery.TokenEndpoint ==
"" || discovery.JWKSURI == "" {
+ return nil, fmt.Errorf("OIDC provider %q discovery is missing
required endpoints", id)
+ }
+ provider := &oidcProvider{
+ id: id, displayName: cfg.DisplayName, issuer: cfg.Issuer,
clientID: cfg.ClientID,
+ postLoginRedirectURL: cfg.PostLoginRedirectURL, discovery:
discovery, httpClient: client,
+ }
+ provider.oauth = oauth2.Config{
+ ClientID: cfg.ClientID, ClientSecret: cfg.ClientSecret,
RedirectURL: cfg.RedirectURL,
+ Scopes: append([]string(nil), cfg.Scopes...),
+ Endpoint: oauth2.Endpoint{AuthURL:
discovery.AuthorizationEndpoint, TokenURL: discovery.TokenEndpoint},
+ }
+ return provider, nil
+}
+
+func (p *oidcProvider) ID() string { return p.id }
+func (p *oidcProvider) DisplayName() string { return p.displayName }
+func (p *oidcProvider) NeedsNonce() bool { return true }
+func (p *oidcProvider) PostLoginRedirectURL() string { return
p.postLoginRedirectURL }
+func (p *oidcProvider) AuthorizationURL(transaction OAuthTransaction) string {
+ return p.oauth.AuthCodeURL(transaction.State,
+ oauth2.SetAuthURLParam("code_challenge",
PKCEChallenge(transaction.CodeVerifier)),
+ oauth2.SetAuthURLParam("code_challenge_method", "S256"),
+ oauth2.SetAuthURLParam("nonce", transaction.Nonce))
+}
+
+func (p *oidcProvider) Authenticate(ctx context.Context, code, codeVerifier,
nonce string) (Principal, error) {
+ ctx = context.WithValue(ctx, oauth2.HTTPClient, p.httpClient)
+ token, err := p.oauth.Exchange(ctx, code,
oauth2.SetAuthURLParam("code_verifier", codeVerifier))
+ if err != nil {
+ return Principal{}, fmt.Errorf("exchange OIDC authorization
code: %w", err)
+ }
+ rawIDToken, ok := token.Extra("id_token").(string)
+ if !ok || rawIDToken == "" {
+ return Principal{}, errors.New("OIDC token response is missing
id_token")
+ }
+ claims, profile, err := p.verifyIDToken(ctx, rawIDToken)
+ if err != nil {
+ return Principal{}, err
+ }
+ if subtle.ConstantTimeCompare([]byte(profile.Nonce), []byte(nonce)) !=
1 {
+ return Principal{}, errors.New("OIDC ID Token nonce does not
match OAuth transaction")
+ }
+ if claims.Subject == "" {
+ return Principal{}, errors.New("OIDC ID Token subject is
missing")
+ }
+ if oidcUsername(profile, claims.Subject) == "" || profile.Email == "" {
+ if p.discovery.UserInfoEndpoint != "" {
+ var userInfo struct {
+ Subject string `json:"sub"`
+ oidcProfile
+ }
+ if err := getOIDCJSON(ctx, p.httpClient,
p.discovery.UserInfoEndpoint, token.AccessToken, &userInfo); err != nil {
+ return Principal{}, fmt.Errorf("read OIDC
UserInfo: %w", err)
+ }
+ if userInfo.Subject != claims.Subject {
+ return Principal{}, errors.New("OIDC UserInfo
subject does not match ID Token subject")
+ }
+ mergeOIDCProfile(&profile, userInfo.oidcProfile)
+ }
+ }
+ return Principal{
+ Subject: p.id + ":" + claims.Subject, Username:
oidcUsername(profile, claims.Subject), Email: profile.Email,
+ Groups: nonNilStrings(profile.Groups), Roles:
nonNilStrings(profile.Roles), AuthType: "oidc", Provider: p.id,
+ }, nil
+}
+
+func (p *oidcProvider) verifyIDToken(ctx context.Context, raw string)
(josejwt.Claims, oidcProfile, error) {
+ token, err := josejwt.ParseSigned(raw,
[]jose.SignatureAlgorithm{jose.RS256})
+ if err != nil {
+ return josejwt.Claims{}, oidcProfile{}, fmt.Errorf("parse OIDC
ID Token: %w", err)
+ }
+ if len(token.Headers) != 1 || token.Headers[0].KeyID == "" {
+ return josejwt.Claims{}, oidcProfile{}, errors.New("OIDC ID
Token kid is missing")
+ }
+ var keySet jose.JSONWebKeySet
+ if err := getOIDCJSON(ctx, p.httpClient, p.discovery.JWKSURI, "",
&keySet); err != nil {
+ return josejwt.Claims{}, oidcProfile{}, fmt.Errorf("read OIDC
JWKS: %w", err)
+ }
+ keys := keySet.Key(token.Headers[0].KeyID)
+ if len(keys) != 1 || keys[0].Algorithm != string(jose.RS256) {
+ return josejwt.Claims{}, oidcProfile{}, errors.New("OIDC ID
Token signing key is unknown or not RS256")
+ }
+ var claims josejwt.Claims
+ var profile oidcProfile
+ if err := token.Claims(keys[0].Key, &claims, &profile); err != nil {
+ return josejwt.Claims{}, oidcProfile{}, fmt.Errorf("verify OIDC
ID Token signature: %w", err)
+ }
+ if claims.Expiry == nil {
+ return josejwt.Claims{}, oidcProfile{}, errors.New("OIDC ID
Token expiration is missing")
+ }
+ if err := claims.ValidateWithLeeway(josejwt.Expected{
+ Issuer: p.issuer, AnyAudience: josejwt.Audience{p.clientID},
Time: time.Now(),
+ }, 0); err != nil {
+ return josejwt.Claims{}, oidcProfile{}, fmt.Errorf("validate
OIDC ID Token issuer, audience, or expiration: %w", err)
Review Comment:
`AnyAudience` accepts a multi-audience ID Token as long as this client ID
appears anywhere, but the implementation never validates the OIDC `azp` claim.
A token whose authorized party is another client can therefore be accepted.
Decode `azp`, require it for multi-audience tokens, and reject any present
value that differs from `p.clientID`.
##########
pkg/config/console/config.go:
##########
@@ -72,6 +78,9 @@ func (c *Config) Validate() error {
if err := c.Auth.Validate(); err != nil {
return err
}
+ if c.GinMode == ReleaseMode && len(c.Auth.Providers) > 0 &&
c.Auth.SessionSecret == auth.DefaultSessionSecret {
+ return bizerror.New(bizerror.ConfigError, "auth sessionSecret
must be explicitly configured when providers are enabled in release mode")
+ }
Review Comment:
This production check rejects only the literal legacy default, so values
such as `sessionSecret: x` are accepted. The secret signs the cookie containing
the authenticated `Principal`; a low-entropy key can be brute-forced and then
used to forge an admin session. Require an adequate minimum key length for
provider-enabled release deployments.
##########
pkg/console/auth/github.go:
##########
@@ -0,0 +1,146 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package auth
+
+import (
+ "context"
+ "encoding/json"
+ "errors"
+ "fmt"
+ "net/http"
+ "strings"
+
+ configauth "github.com/apache/dubbo-admin/pkg/config/console/auth"
+ "golang.org/x/oauth2"
+ "golang.org/x/oauth2/github"
+)
+
+const githubAPIBaseURL = "https://api.github.com";
+
+type githubEndpoints struct {
+ OAuth oauth2.Endpoint
+ APIBaseURL string
+}
+
+type githubProvider struct {
+ id string
+ displayName string
+ postLoginRedirectURL string
+ oauth oauth2.Config
+ apiBaseURL string
+ httpClient *http.Client
+}
+
+type githubUser struct {
+ ID int64 `json:"id"`
+ Login string `json:"login"`
+ Email string `json:"email"`
+}
+
+type githubEmail struct {
+ Email string `json:"email"`
+ Primary bool `json:"primary"`
+ Verified bool `json:"verified"`
+}
+
+func NewGitHubProvider(id string, cfg configauth.ProviderConfig) Provider {
+ return newGitHubProvider(id, cfg, githubEndpoints{OAuth:
github.Endpoint, APIBaseURL: githubAPIBaseURL}, http.DefaultClient)
+}
+
+func newGitHubProvider(id string, cfg configauth.ProviderConfig, endpoints
githubEndpoints, client *http.Client) Provider {
+ return &githubProvider{
+ id: id,
+ displayName: cfg.DisplayName,
+ postLoginRedirectURL: cfg.PostLoginRedirectURL,
+ oauth: oauth2.Config{
+ ClientID: cfg.ClientID, ClientSecret: cfg.ClientSecret,
RedirectURL: cfg.RedirectURL,
+ Scopes: append([]string(nil), cfg.Scopes...), Endpoint:
endpoints.OAuth,
+ },
+ apiBaseURL: strings.TrimRight(endpoints.APIBaseURL, "/"),
+ httpClient: client,
+ }
+}
+
+func (p *githubProvider) ID() string { return p.id }
+func (p *githubProvider) DisplayName() string { return p.displayName }
+func (p *githubProvider) NeedsNonce() bool { return false }
+func (p *githubProvider) PostLoginRedirectURL() string { return
p.postLoginRedirectURL }
+func (p *githubProvider) AuthorizationURL(transaction OAuthTransaction) string
{
+ return p.oauth.AuthCodeURL(transaction.State,
+ oauth2.SetAuthURLParam("code_challenge",
PKCEChallenge(transaction.CodeVerifier)),
+ oauth2.SetAuthURLParam("code_challenge_method", "S256"))
+}
+
+func (p *githubProvider) Authenticate(ctx context.Context, code, codeVerifier,
_ string) (Principal, error) {
+ ctx = context.WithValue(ctx, oauth2.HTTPClient, p.httpClient)
+ token, err := p.oauth.Exchange(ctx, code,
oauth2.SetAuthURLParam("code_verifier", codeVerifier))
+ if err != nil {
+ return Principal{}, fmt.Errorf("exchange GitHub authorization
code: %w", err)
+ }
+ client := p.oauth.Client(ctx, token)
+ var user githubUser
+ if err := getGitHubJSON(ctx, client, p.apiBaseURL+"/user", &user); err
!= nil {
+ return Principal{}, fmt.Errorf("decode GitHub user: %w", err)
+ }
+ if user.ID <= 0 {
+ return Principal{}, errors.New("GitHub user numeric id is
missing")
+ }
+ email := user.Email
+ if email == "" {
+ var emails []githubEmail
+ if err := getGitHubJSON(ctx, client,
p.apiBaseURL+"/user/emails", &emails); err != nil {
+ return Principal{}, fmt.Errorf("decode GitHub emails:
%w", err)
+ }
Review Comment:
Custom GitHub scopes are allowed, but `/user/emails` requires the
`user:email` scope. For users with no public email, a valid configuration such
as `scopes: [read:user]` therefore reaches this call, receives an authorization
error, and rejects the entire login even though `Principal.Email` may be empty.
Either require `user:email` during validation or skip this fallback when the
scope was not granted.
##########
pkg/console/auth/oidc.go:
##########
@@ -0,0 +1,229 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package auth
+
+import (
+ "context"
+ "crypto/subtle"
+ "encoding/json"
+ "errors"
+ "fmt"
+ "net/http"
+ "strings"
+ "time"
+
+ configauth "github.com/apache/dubbo-admin/pkg/config/console/auth"
+ jose "github.com/go-jose/go-jose/v4"
+ josejwt "github.com/go-jose/go-jose/v4/jwt"
+ "golang.org/x/oauth2"
+)
+
+type oidcDiscovery struct {
+ Issuer string `json:"issuer"`
+ AuthorizationEndpoint string `json:"authorization_endpoint"`
+ TokenEndpoint string `json:"token_endpoint"`
+ JWKSURI string `json:"jwks_uri"`
+ UserInfoEndpoint string `json:"userinfo_endpoint"`
+}
+
+type oidcProfile struct {
+ Nonce string `json:"nonce"`
+ PreferredUsername string `json:"preferred_username"`
+ Name string `json:"name"`
+ Email string `json:"email"`
+ Groups []string `json:"groups"`
+ Roles []string `json:"roles"`
+}
+
+type oidcProvider struct {
+ id string
+ displayName string
+ issuer string
+ clientID string
+ postLoginRedirectURL string
+ discovery oidcDiscovery
+ oauth oauth2.Config
+ httpClient *http.Client
+}
+
+func NewOIDCProvider(ctx context.Context, id string, cfg
configauth.ProviderConfig, client *http.Client) (Provider, error) {
+ if client == nil {
+ client = http.DefaultClient
+ }
Review Comment:
`http.DefaultClient` has no overall timeout. OIDC discovery runs
synchronously from the core Console component's `Start`, so an issuer that
accepts a connection but never responds can hang Console startup indefinitely
instead of returning a startup error. Use a bounded client for discovery and
subsequent provider requests.
##########
pkg/console/auth/oidc.go:
##########
@@ -0,0 +1,229 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package auth
+
+import (
+ "context"
+ "crypto/subtle"
+ "encoding/json"
+ "errors"
+ "fmt"
+ "net/http"
+ "strings"
+ "time"
+
+ configauth "github.com/apache/dubbo-admin/pkg/config/console/auth"
+ jose "github.com/go-jose/go-jose/v4"
+ josejwt "github.com/go-jose/go-jose/v4/jwt"
+ "golang.org/x/oauth2"
+)
+
+type oidcDiscovery struct {
+ Issuer string `json:"issuer"`
+ AuthorizationEndpoint string `json:"authorization_endpoint"`
+ TokenEndpoint string `json:"token_endpoint"`
+ JWKSURI string `json:"jwks_uri"`
+ UserInfoEndpoint string `json:"userinfo_endpoint"`
+}
+
+type oidcProfile struct {
+ Nonce string `json:"nonce"`
+ PreferredUsername string `json:"preferred_username"`
+ Name string `json:"name"`
+ Email string `json:"email"`
+ Groups []string `json:"groups"`
+ Roles []string `json:"roles"`
+}
+
+type oidcProvider struct {
+ id string
+ displayName string
+ issuer string
+ clientID string
+ postLoginRedirectURL string
+ discovery oidcDiscovery
+ oauth oauth2.Config
+ httpClient *http.Client
+}
+
+func NewOIDCProvider(ctx context.Context, id string, cfg
configauth.ProviderConfig, client *http.Client) (Provider, error) {
+ if client == nil {
+ client = http.DefaultClient
+ }
+ discoveryURL := strings.TrimRight(cfg.Issuer, "/") +
"/.well-known/openid-configuration"
+ var discovery oidcDiscovery
+ if err := getOIDCJSON(ctx, client, discoveryURL, "", &discovery); err
!= nil {
+ return nil, fmt.Errorf("discover OIDC provider %q: %w", id, err)
+ }
+ if discovery.Issuer != cfg.Issuer {
+ return nil, fmt.Errorf("OIDC provider %q discovery issuer %q
does not match configured issuer %q", id, discovery.Issuer, cfg.Issuer)
+ }
+ if discovery.AuthorizationEndpoint == "" || discovery.TokenEndpoint ==
"" || discovery.JWKSURI == "" {
+ return nil, fmt.Errorf("OIDC provider %q discovery is missing
required endpoints", id)
+ }
+ provider := &oidcProvider{
+ id: id, displayName: cfg.DisplayName, issuer: cfg.Issuer,
clientID: cfg.ClientID,
+ postLoginRedirectURL: cfg.PostLoginRedirectURL, discovery:
discovery, httpClient: client,
+ }
+ provider.oauth = oauth2.Config{
+ ClientID: cfg.ClientID, ClientSecret: cfg.ClientSecret,
RedirectURL: cfg.RedirectURL,
+ Scopes: append([]string(nil), cfg.Scopes...),
+ Endpoint: oauth2.Endpoint{AuthURL:
discovery.AuthorizationEndpoint, TokenURL: discovery.TokenEndpoint},
+ }
+ return provider, nil
+}
+
+func (p *oidcProvider) ID() string { return p.id }
+func (p *oidcProvider) DisplayName() string { return p.displayName }
+func (p *oidcProvider) NeedsNonce() bool { return true }
+func (p *oidcProvider) PostLoginRedirectURL() string { return
p.postLoginRedirectURL }
+func (p *oidcProvider) AuthorizationURL(transaction OAuthTransaction) string {
+ return p.oauth.AuthCodeURL(transaction.State,
+ oauth2.SetAuthURLParam("code_challenge",
PKCEChallenge(transaction.CodeVerifier)),
+ oauth2.SetAuthURLParam("code_challenge_method", "S256"),
+ oauth2.SetAuthURLParam("nonce", transaction.Nonce))
+}
+
+func (p *oidcProvider) Authenticate(ctx context.Context, code, codeVerifier,
nonce string) (Principal, error) {
+ ctx = context.WithValue(ctx, oauth2.HTTPClient, p.httpClient)
+ token, err := p.oauth.Exchange(ctx, code,
oauth2.SetAuthURLParam("code_verifier", codeVerifier))
+ if err != nil {
+ return Principal{}, fmt.Errorf("exchange OIDC authorization
code: %w", err)
+ }
+ rawIDToken, ok := token.Extra("id_token").(string)
+ if !ok || rawIDToken == "" {
+ return Principal{}, errors.New("OIDC token response is missing
id_token")
+ }
+ claims, profile, err := p.verifyIDToken(ctx, rawIDToken)
+ if err != nil {
+ return Principal{}, err
+ }
+ if subtle.ConstantTimeCompare([]byte(profile.Nonce), []byte(nonce)) !=
1 {
+ return Principal{}, errors.New("OIDC ID Token nonce does not
match OAuth transaction")
+ }
+ if claims.Subject == "" {
+ return Principal{}, errors.New("OIDC ID Token subject is
missing")
+ }
+ if oidcUsername(profile, claims.Subject) == "" || profile.Email == "" {
+ if p.discovery.UserInfoEndpoint != "" {
+ var userInfo struct {
+ Subject string `json:"sub"`
+ oidcProfile
+ }
+ if err := getOIDCJSON(ctx, p.httpClient,
p.discovery.UserInfoEndpoint, token.AccessToken, &userInfo); err != nil {
+ return Principal{}, fmt.Errorf("read OIDC
UserInfo: %w", err)
+ }
+ if userInfo.Subject != claims.Subject {
+ return Principal{}, errors.New("OIDC UserInfo
subject does not match ID Token subject")
+ }
+ mergeOIDCProfile(&profile, userInfo.oidcProfile)
+ }
+ }
+ return Principal{
+ Subject: p.id + ":" + claims.Subject, Username:
oidcUsername(profile, claims.Subject), Email: profile.Email,
+ Groups: nonNilStrings(profile.Groups), Roles:
nonNilStrings(profile.Roles), AuthType: "oidc", Provider: p.id,
+ }, nil
+}
+
+func (p *oidcProvider) verifyIDToken(ctx context.Context, raw string)
(josejwt.Claims, oidcProfile, error) {
+ token, err := josejwt.ParseSigned(raw,
[]jose.SignatureAlgorithm{jose.RS256})
+ if err != nil {
+ return josejwt.Claims{}, oidcProfile{}, fmt.Errorf("parse OIDC
ID Token: %w", err)
+ }
+ if len(token.Headers) != 1 || token.Headers[0].KeyID == "" {
+ return josejwt.Claims{}, oidcProfile{}, errors.New("OIDC ID
Token kid is missing")
+ }
+ var keySet jose.JSONWebKeySet
+ if err := getOIDCJSON(ctx, p.httpClient, p.discovery.JWKSURI, "",
&keySet); err != nil {
+ return josejwt.Claims{}, oidcProfile{}, fmt.Errorf("read OIDC
JWKS: %w", err)
+ }
+ keys := keySet.Key(token.Headers[0].KeyID)
+ if len(keys) != 1 || keys[0].Algorithm != string(jose.RS256) {
+ return josejwt.Claims{}, oidcProfile{}, errors.New("OIDC ID
Token signing key is unknown or not RS256")
+ }
Review Comment:
A JWK's `alg` member is optional, but this rejects an otherwise valid RS256
key whenever the provider omits it. Since `ParseSigned` already restricts the
token header to RS256, accept an empty JWK algorithm while still rejecting a
conflicting declared algorithm.
##########
pkg/console/handler/auth.go:
##########
@@ -18,53 +18,159 @@
package handler
import (
+ "errors"
"net/http"
+ "slices"
"github.com/gin-contrib/sessions"
"github.com/gin-gonic/gin"
"github.com/apache/dubbo-admin/pkg/common/bizerror"
+ configauth "github.com/apache/dubbo-admin/pkg/config/console/auth"
+ consoleauth "github.com/apache/dubbo-admin/pkg/console/auth"
consolectx "github.com/apache/dubbo-admin/pkg/console/context"
"github.com/apache/dubbo-admin/pkg/console/model"
)
-func Login(ctx consolectx.Context) gin.HandlerFunc {
- return func(c *gin.Context) {
- user := c.PostForm("user")
- password := c.PostForm("password")
- // verify username and password
- authCfg := ctx.Config().Console.Auth
- if user != authCfg.User || password != authCfg.Password {
- authErr := bizerror.New(bizerror.Unauthorized,
"username or password is not correct!")
- c.JSON(http.StatusUnauthorized,
model.NewBizErrorResp(authErr))
- return
- }
- session := sessions.Default(c)
- session.Set("user", user)
- session.Options(sessions.Options{
- MaxAge: authCfg.ExpirationTime,
- Path: "/",
- })
- err := session.Save()
- if err != nil {
- sessionErr := bizerror.New(bizerror.SessionError,
err.Error())
- c.JSON(http.StatusOK, model.NewBizErrorResp(sessionErr))
- return
- }
- c.JSON(http.StatusOK, model.NewSuccessResp(true))
+type AuthHandler struct {
+ config *configauth.Config
+ service *consoleauth.Service
+}
+
+type providersResponse struct {
+ Methods []string `json:"methods"`
+ Providers []consoleauth.PublicProvider `json:"providers"`
+}
+
+func NewAuthHandler(ctx consolectx.Context) (*AuthHandler, error) {
+ config := ctx.Config().Console.Auth
+ service, err := consoleauth.NewService(ctx.AppContext(),
config.Providers, nil)
+ if err != nil {
+ return nil, err
}
+ return newAuthHandler(config, service), nil
+}
+
+func newAuthHandler(config *configauth.Config, service *consoleauth.Service)
*AuthHandler {
+ return &AuthHandler{config: config, service: service}
}
-func Logout(_ consolectx.Context) gin.HandlerFunc {
- return func(c *gin.Context) {
- session := sessions.Default(c)
- session.Clear()
- err := session.Save()
- if err != nil {
- sessionErr := bizerror.New(bizerror.SessionError,
err.Error())
- c.JSON(http.StatusOK, model.NewBizErrorResp(sessionErr))
- return
- }
- c.JSON(http.StatusOK, model.NewSuccessResp(true))
+func (h *AuthHandler) Login(c *gin.Context) {
+ if !slices.Contains(h.config.Methods, configauth.MethodPassword) {
+ c.JSON(http.StatusNotFound,
model.NewBizErrorResp(bizerror.New(bizerror.NotFoundError, "password login is
not enabled")))
+ return
+ }
+ user := c.PostForm("user")
+ password := c.PostForm("password")
+ if user != h.config.User || password != h.config.Password {
+ c.JSON(http.StatusUnauthorized,
model.NewBizErrorResp(bizerror.New(bizerror.Unauthorized, "username or password
is not correct!")))
+ return
+ }
+ session := sessions.Default(c)
+ if err := consoleauth.PutPrincipal(session,
consoleauth.LocalPrincipal(user)); err != nil {
+ writeSessionError(c, err)
+ return
+ }
+ if err := session.Save(); err != nil {
+ writeSessionError(c, err)
+ return
+ }
+ c.JSON(http.StatusOK, model.NewSuccessResp(true))
+}
+
+func (h *AuthHandler) Logout(c *gin.Context) {
+ session := sessions.Default(c)
+ session.Clear()
+ session.Options(sessions.Options{
+ Path: "/", MaxAge: -1, Secure: h.config.SessionCookieSecure,
HttpOnly: true, SameSite: http.SameSiteLaxMode,
+ })
+ if err := session.Save(); err != nil {
+ writeSessionError(c, err)
+ return
+ }
+ c.JSON(http.StatusOK, model.NewSuccessResp(true))
+}
+
+func (h *AuthHandler) Providers(c *gin.Context) {
+ c.JSON(http.StatusOK, model.NewSuccessResp(providersResponse{
+ Methods: append([]string(nil), h.config.Methods...), Providers:
h.service.PublicProviders(),
+ }))
+}
+
+func (h *AuthHandler) ProviderLogin(c *gin.Context) {
+ transaction, authorizationURL, err :=
h.service.Begin(c.Param("provider"))
+ if err != nil {
+ writeProviderError(c, err)
+ return
+ }
+ session := sessions.Default(c)
+ if err := consoleauth.PutOAuthTransaction(session, transaction); err !=
nil {
+ writeSessionError(c, err)
+ return
+ }
+ if err := session.Save(); err != nil {
+ writeSessionError(c, err)
+ return
}
+ c.Redirect(http.StatusFound, authorizationURL)
+}
+
+func (h *AuthHandler) ProviderCallback(c *gin.Context) {
+ session := sessions.Default(c)
+ transaction, err := consoleauth.ConsumeOAuthTransaction(session)
+ if err != nil {
+ c.JSON(http.StatusBadRequest,
model.NewBizErrorResp(bizerror.New(bizerror.InvalidArgument, err.Error())))
+ return
+ }
+ // Persist consumption before contacting the Provider so failures
cannot be replayed.
+ if err := session.Save(); err != nil {
+ writeSessionError(c, err)
+ return
+ }
Review Comment:
With `cookie.NewStore`, deleting and saving the transaction only sends a
replacement cookie; it does not revoke the original client-held cookie. A
concurrent request or a client replaying the login response's original cookie
reconstructs the transaction and reaches `Complete` again, so the transaction
is not server-side single-use (the test only retries with the updated cookie).
Store transactions in a server-side cache keyed by state and consume them
atomically, then test replay with the original cookie.
##########
pkg/config/console/auth/config.go:
##########
@@ -19,26 +19,136 @@ package auth
import (
"errors"
+ "fmt"
+ "net/url"
+ "regexp"
+ "slices"
"github.com/apache/dubbo-admin/pkg/config"
)
-const DefaultExpirationTime = 7200
+const (
+ DefaultExpirationTime = 7200
+ DefaultSessionSecret = "secret"
+
+ MethodPassword = "password"
+ ProviderTypeGitHub = "github"
+ ProviderTypeOIDC = "oidc"
+)
+
+var providerIDPattern = regexp.MustCompile(`^[A-Za-z0-9][A-Za-z0-9._-]{0,63}$`)
+
+type ProviderConfig struct {
+ Type string `json:"type" yaml:"type"`
+ DisplayName string `json:"displayName" yaml:"displayName"`
+ Issuer string `json:"issuer,omitempty"
yaml:"issuer,omitempty"`
+ ClientID string `json:"clientId" yaml:"clientId"`
+ ClientSecret string `json:"clientSecret" yaml:"clientSecret"`
+ RedirectURL string `json:"redirectUrl" yaml:"redirectUrl"`
+ PostLoginRedirectURL string `json:"postLoginRedirectUrl"
yaml:"postLoginRedirectUrl"`
+ Scopes []string `json:"scopes,omitempty"
yaml:"scopes,omitempty"`
+}
// Config AuthConfig configure the valid user and password
type Config struct {
config.BaseConfig
- User string `json:"user"`
- Password string `json:"password"`
- ExpirationTime int `json:"expirationTime"`
+ Methods []string `json:"methods"
yaml:"methods"`
+ User string `json:"user" yaml:"user"`
+ Password string `json:"password"
yaml:"password"`
+ ExpirationTime int `json:"expirationTime"
yaml:"expirationTime"`
+ SessionSecret string `json:"sessionSecret"
yaml:"sessionSecret"`
+ SessionCookieSecure bool
`json:"sessionCookieSecure" yaml:"sessionCookieSecure"`
+ Providers map[string]ProviderConfig
`json:"providers,omitempty" yaml:"providers,omitempty"`
+}
+
+func (c *Config) Sanitize() {
+ c.Password = config.SanitizedValue
+ c.SessionSecret = config.SanitizedValue
+ for id, provider := range c.Providers {
+ provider.ClientSecret = config.SanitizedValue
+ c.Providers[id] = provider
+ }
}
func (c *Config) Validate() error {
- if c.User == "" || c.Password == "" {
+ if len(c.Methods) == 0 {
+ c.Methods = []string{MethodPassword}
+ }
Review Comment:
An explicitly empty `methods` list is indistinguishable here from an omitted
list, so validation re-enables password login. Because `password` is the only
accepted method, provider-only deployments cannot actually disable the password
endpoint as advertised. Default only a nil (omitted) slice; preserve a non-nil
empty slice.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
