Ver código fonte

refactor(oauth2): apply Go conventions and add GoDoc comments

Remove Get prefix from Provider interface methods, rename Profile
struct to UserProfile to avoid method name collision, fix acronym
casing (authUrl → authURL), fix receiver naming, and return
interfaces from constructors instead of unexported concrete types.
Frédéric Guillot 1 semana atrás
pai
commit
062641330a

+ 14 - 8
internal/oauth2/authorization.go

@@ -12,38 +12,44 @@ import (
 	"miniflux.app/v2/internal/crypto"
 )
 
+// Authorization holds the OAuth2 authorization URL, state parameter, and PKCE code verifier.
 type Authorization struct {
 	url          string
 	state        string
 	codeVerifier string
 }
 
-func (u *Authorization) RedirectURL() string {
-	return u.url
+// RedirectURL returns the OAuth2 authorization URL to redirect the user to.
+func (a *Authorization) RedirectURL() string {
+	return a.url
 }
 
-func (u *Authorization) State() string {
-	return u.state
+// State returns the random state parameter used for CSRF protection.
+func (a *Authorization) State() string {
+	return a.state
 }
 
-func (u *Authorization) CodeVerifier() string {
-	return u.codeVerifier
+// CodeVerifier returns the PKCE code verifier associated with this authorization.
+func (a *Authorization) CodeVerifier() string {
+	return a.codeVerifier
 }
 
+// GenerateAuthorization creates a new Authorization with a random state and PKCE code challenge
+// derived from the given OAuth2 configuration.
 func GenerateAuthorization(config *oauth2.Config) *Authorization {
 	codeVerifier := crypto.GenerateRandomStringHex(32)
 	sum := sha256.Sum256([]byte(codeVerifier))
 
 	state := crypto.GenerateRandomStringHex(24)
 
-	authUrl := config.AuthCodeURL(
+	authURL := config.AuthCodeURL(
 		state,
 		oauth2.SetAuthURLParam("code_challenge_method", "S256"),
 		oauth2.SetAuthURLParam("code_challenge", base64.RawURLEncoding.EncodeToString(sum[:])),
 	)
 
 	return &Authorization{
-		url:          authUrl,
+		url:          authURL,
 		state:        state,
 		codeVerifier: codeVerifier,
 	}

+ 10 - 10
internal/oauth2/google.go

@@ -32,11 +32,12 @@ type googleProvider struct {
 	redirectURL  string
 }
 
-func NewGoogleProvider(clientID, clientSecret, redirectURL string) *googleProvider {
+// NewGoogleProvider returns a Provider that authenticates users via Google OAuth2.
+func NewGoogleProvider(clientID, clientSecret, redirectURL string) Provider {
 	return &googleProvider{clientID: clientID, clientSecret: clientSecret, redirectURL: redirectURL}
 }
 
-func (g *googleProvider) GetConfig() *oauth2.Config {
+func (g *googleProvider) Config() *oauth2.Config {
 	return &oauth2.Config{
 		RedirectURL:  g.redirectURL,
 		ClientID:     g.clientID,
@@ -49,12 +50,12 @@ func (g *googleProvider) GetConfig() *oauth2.Config {
 	}
 }
 
-func (g *googleProvider) GetUserExtraKey() string {
+func (g *googleProvider) UserExtraKey() string {
 	return "google_id"
 }
 
-func (g *googleProvider) GetProfile(ctx context.Context, code, codeVerifier string) (*Profile, error) {
-	conf := g.GetConfig()
+func (g *googleProvider) Profile(ctx context.Context, code, codeVerifier string) (*UserProfile, error) {
+	conf := g.Config()
 	token, err := conf.Exchange(ctx, code, oauth2.SetAuthURLParam("code_verifier", codeVerifier))
 	if err != nil {
 		return nil, fmt.Errorf("google: failed to exchange token: %w", err)
@@ -77,19 +78,18 @@ func (g *googleProvider) GetProfile(ctx context.Context, code, codeVerifier stri
 		return nil, fmt.Errorf("google: unable to unserialize Google profile: %w", err)
 	}
 
-	profile := &Profile{Key: g.GetUserExtraKey(), ID: user.Sub, Username: user.Email}
-	return profile, nil
+	return &UserProfile{Key: g.UserExtraKey(), ID: user.Sub, Username: user.Email}, nil
 }
 
-func (g *googleProvider) PopulateUserCreationWithProfileID(user *model.UserCreationRequest, profile *Profile) {
+func (g *googleProvider) PopulateUserCreationWithProfileID(user *model.UserCreationRequest, profile *UserProfile) {
 	user.GoogleID = profile.ID
 }
 
-func (g *googleProvider) PopulateUserWithProfileID(user *model.User, profile *Profile) {
+func (g *googleProvider) PopulateUserWithProfileID(user *model.User, profile *UserProfile) {
 	user.GoogleID = profile.ID
 }
 
-func (g *googleProvider) GetUserProfileID(user *model.User) string {
+func (g *googleProvider) UserProfileID(user *model.User) string {
 	return user.GoogleID
 }
 

+ 6 - 0
internal/oauth2/manager.go

@@ -9,10 +9,13 @@ import (
 	"log/slog"
 )
 
+// Manager manages registered OAuth2 providers.
 type Manager struct {
 	providers map[string]Provider
 }
 
+// FindProvider returns the provider registered under the given name,
+// or an error if no such provider exists.
 func (m *Manager) FindProvider(name string) (Provider, error) {
 	if provider, found := m.providers[name]; found {
 		return provider, nil
@@ -21,10 +24,13 @@ func (m *Manager) FindProvider(name string) (Provider, error) {
 	return nil, errors.New("oauth2 provider not found")
 }
 
+// AddProvider registers a provider under the given name.
 func (m *Manager) AddProvider(name string, provider Provider) {
 	m.providers[name] = provider
 }
 
+// NewManager creates a Manager and registers either an OIDC provider (if a discovery
+// endpoint is provided) or a Google provider as the default.
 func NewManager(ctx context.Context, clientID, clientSecret, redirectURL, oidcDiscoveryEndpoint string) *Manager {
 	m := &Manager{providers: make(map[string]Provider)}
 

+ 21 - 20
internal/oauth2/oidc.go

@@ -14,9 +14,15 @@ import (
 	"golang.org/x/oauth2"
 )
 
-var (
-	ErrEmptyUsername = errors.New("oidc: username is empty")
-)
+// ErrEmptyUsername is returned when the OIDC user profile has no username.
+var ErrEmptyUsername = errors.New("oidc: username is empty")
+
+type userClaims struct {
+	Email             string `json:"email"`
+	Profile           string `json:"profile"`
+	Name              string `json:"name"`
+	PreferredUsername string `json:"preferred_username"`
+}
 
 type oidcProvider struct {
 	clientID     string
@@ -25,7 +31,9 @@ type oidcProvider struct {
 	provider     *oidc.Provider
 }
 
-func NewOidcProvider(ctx context.Context, clientID, clientSecret, redirectURL, discoveryEndpoint string) (*oidcProvider, error) {
+// NewOidcProvider returns a Provider that authenticates users via OpenID Connect.
+// It discovers the OIDC endpoints from the given discovery URL.
+func NewOidcProvider(ctx context.Context, clientID, clientSecret, redirectURL, discoveryEndpoint string) (Provider, error) {
 	provider, err := oidc.NewProvider(ctx, discoveryEndpoint)
 	if err != nil {
 		return nil, fmt.Errorf(`oidc: failed to initialize provider %q: %w`, discoveryEndpoint, err)
@@ -39,11 +47,11 @@ func NewOidcProvider(ctx context.Context, clientID, clientSecret, redirectURL, d
 	}, nil
 }
 
-func (o *oidcProvider) GetUserExtraKey() string {
+func (o *oidcProvider) UserExtraKey() string {
 	return "openid_connect_id"
 }
 
-func (o *oidcProvider) GetConfig() *oauth2.Config {
+func (o *oidcProvider) Config() *oauth2.Config {
 	return &oauth2.Config{
 		RedirectURL:  o.redirectURL,
 		ClientID:     o.clientID,
@@ -53,8 +61,8 @@ func (o *oidcProvider) GetConfig() *oauth2.Config {
 	}
 }
 
-func (o *oidcProvider) GetProfile(ctx context.Context, code, codeVerifier string) (*Profile, error) {
-	conf := o.GetConfig()
+func (o *oidcProvider) Profile(ctx context.Context, code, codeVerifier string) (*UserProfile, error) {
+	conf := o.Config()
 	token, err := conf.Exchange(ctx, code, oauth2.SetAuthURLParam("code_verifier", codeVerifier))
 	if err != nil {
 		return nil, fmt.Errorf(`oidc: failed to exchange token: %w`, err)
@@ -80,8 +88,8 @@ func (o *oidcProvider) GetProfile(ctx context.Context, code, codeVerifier string
 		return nil, fmt.Errorf(`oidc: id token subject %q does not match userinfo subject %q`, idToken.Subject, userInfo.Subject)
 	}
 
-	profile := &Profile{
-		Key: o.GetUserExtraKey(),
+	profile := &UserProfile{
+		Key: o.UserExtraKey(),
 		ID:  userInfo.Subject,
 	}
 
@@ -106,25 +114,18 @@ func (o *oidcProvider) GetProfile(ctx context.Context, code, codeVerifier string
 	return profile, nil
 }
 
-func (o *oidcProvider) PopulateUserCreationWithProfileID(user *model.UserCreationRequest, profile *Profile) {
+func (o *oidcProvider) PopulateUserCreationWithProfileID(user *model.UserCreationRequest, profile *UserProfile) {
 	user.OpenIDConnectID = profile.ID
 }
 
-func (o *oidcProvider) PopulateUserWithProfileID(user *model.User, profile *Profile) {
+func (o *oidcProvider) PopulateUserWithProfileID(user *model.User, profile *UserProfile) {
 	user.OpenIDConnectID = profile.ID
 }
 
-func (o *oidcProvider) GetUserProfileID(user *model.User) string {
+func (o *oidcProvider) UserProfileID(user *model.User) string {
 	return user.OpenIDConnectID
 }
 
 func (o *oidcProvider) UnsetUserProfileID(user *model.User) {
 	user.OpenIDConnectID = ""
 }
-
-type userClaims struct {
-	Email             string `json:"email"`
-	Profile           string `json:"profile"`
-	Name              string `json:"name"`
-	PreferredUsername string `json:"preferred_username"`
-}

+ 4 - 3
internal/oauth2/profile.go

@@ -7,13 +7,14 @@ import (
 	"fmt"
 )
 
-// Profile is the OAuth2 user profile.
-type Profile struct {
+// UserProfile represents a user's profile retrieved from an OAuth2 provider.
+type UserProfile struct {
 	Key      string
 	ID       string
 	Username string
 }
 
-func (p Profile) String() string {
+// String returns a formatted string representation of the user profile.
+func (p UserProfile) String() string {
 	return fmt.Sprintf(`Key=%s ; ID=%s ; Username=%s`, p.Key, p.ID, p.Username)
 }

+ 20 - 7
internal/oauth2/provider.go

@@ -11,13 +11,26 @@ import (
 	"miniflux.app/v2/internal/model"
 )
 
-// Provider is an interface for OAuth2 providers.
+// Provider defines the interface that all OAuth2 providers must implement.
 type Provider interface {
-	GetConfig() *oauth2.Config
-	GetUserExtraKey() string
-	GetProfile(ctx context.Context, code, codeVerifier string) (*Profile, error)
-	PopulateUserCreationWithProfileID(user *model.UserCreationRequest, profile *Profile)
-	PopulateUserWithProfileID(user *model.User, profile *Profile)
-	GetUserProfileID(user *model.User) string
+	// Config returns the OAuth2 configuration for this provider.
+	Config() *oauth2.Config
+
+	// UserExtraKey returns the key used to store the provider-specific user ID.
+	UserExtraKey() string
+
+	// Profile exchanges the authorization code for a token and fetches the user's profile.
+	Profile(ctx context.Context, code, codeVerifier string) (*UserProfile, error)
+
+	// PopulateUserCreationWithProfileID sets the provider-specific ID on a new user creation request.
+	PopulateUserCreationWithProfileID(user *model.UserCreationRequest, profile *UserProfile)
+
+	// PopulateUserWithProfileID sets the provider-specific ID on an existing user.
+	PopulateUserWithProfileID(user *model.User, profile *UserProfile)
+
+	// UserProfileID returns the provider-specific ID from the given user.
+	UserProfileID(user *model.User) string
+
+	// UnsetUserProfileID removes the provider-specific ID from the given user.
 	UnsetUserProfileID(user *model.User)
 }

+ 2 - 2
internal/ui/oauth2_callback.go

@@ -53,7 +53,7 @@ func (h *handler) oauth2Callback(w http.ResponseWriter, r *http.Request) {
 		return
 	}
 
-	profile, err := authProvider.GetProfile(r.Context(), code, request.OAuth2CodeVerifier(r))
+	profile, err := authProvider.Profile(r.Context(), code, request.OAuth2CodeVerifier(r))
 	if err != nil {
 		slog.Warn("Unable to get OAuth2 profile from provider",
 			slog.String("provider", provider),
@@ -87,7 +87,7 @@ func (h *handler) oauth2Callback(w http.ResponseWriter, r *http.Request) {
 			return
 		}
 
-		existingProfileID := authProvider.GetUserProfileID(loggedUser)
+		existingProfileID := authProvider.UserProfileID(loggedUser)
 		if existingProfileID != "" && existingProfileID != profile.ID {
 			slog.Error("Oauth2 user cannot be associated because this user is already linked to a different identity",
 				slog.Int64("user_id", loggedUser.ID),

+ 1 - 1
internal/ui/oauth2_redirect.go

@@ -31,7 +31,7 @@ func (h *handler) oauth2Redirect(w http.ResponseWriter, r *http.Request) {
 		return
 	}
 
-	auth := oauth2.GenerateAuthorization(authProvider.GetConfig())
+	auth := oauth2.GenerateAuthorization(authProvider.Config())
 
 	sess := session.New(h.store, request.SessionID(r))
 	sess.SetOAuth2State(auth.State())