NOISSUE - Use only sign in state in OAuth 2.0 flows (#2270)

Signed-off-by: Rodney Osodo <socials@rodneyosodo.com>

pkg/oauth2/oauth2.go

@@ -5,45 +5,11 @@ package oauth2
 
 import (
 	"context"
-	"errors"
 
 	mfclients "github.com/absmach/magistrala/pkg/clients"
 	"golang.org/x/oauth2"
 )
 
-// State is the state of the OAuth2 flow.
-type State uint8
-
-const (
-	// SignIn is the state for the sign-in flow.
-	SignIn State = iota
-	// SignUp is the state for the sign-up flow.
-	SignUp
-)
-
-func (s State) String() string {
-	switch s {
-	case SignIn:
-		return "signin"
-	case SignUp:
-		return "signup"
-	default:
-		return "unknown"
-	}
-}
-
-// ToState converts string value to a valid OAuth2 state.
-func ToState(state string) (State, error) {
-	switch state {
-	case "signin":
-		return SignIn, nil
-	case "signup":
-		return SignUp, nil
-	}
-
-	return State(0), errors.New("invalid state")
-}
-
 // Config is the configuration for the OAuth2 provider.
 type Config struct {
 	ClientID     string `env:"CLIENT_ID"       envDefault:""`

users/api/clients.go

@@ -536,19 +536,7 @@ func oauth2CallbackHandler(oauth oauth2.Provider, svc users.Service) http.Handle
 			http.Redirect(w, r, oauth.ErrorURL()+"?error=oauth%20provider%20is%20disabled", http.StatusSeeOther)
 			return
 		}
-		// state is prefixed with signin- or signup- to indicate which flow we should use
-		var state string
-		var flow oauth2.State
-		var err error
-		if strings.Contains(r.FormValue("state"), "-") {
-			state = strings.Split(r.FormValue("state"), "-")[1]
-			flow, err = oauth2.ToState(strings.Split(r.FormValue("state"), "-")[0])
-			if err != nil {
-				http.Redirect(w, r, oauth.ErrorURL()+"?error="+err.Error(), http.StatusSeeOther) //nolint:goconst
-				return
-			}
-		}
-
+		state := r.FormValue("state")
 		if state != oauth.State() {
 			http.Redirect(w, r, oauth.ErrorURL()+"?error=invalid%20state", http.StatusSeeOther)
 			return
@@ -567,7 +555,7 @@ func oauth2CallbackHandler(oauth oauth2.Provider, svc users.Service) http.Handle
 				return
 			}
 
-			jwt, err := svc.OAuthCallback(r.Context(), flow, client)
+			jwt, err := svc.OAuthCallback(r.Context(), client)
 			if err != nil {
 				http.Redirect(w, r, oauth.ErrorURL()+"?error="+err.Error(), http.StatusSeeOther)
 				return

users/api/logging.go

@@ -10,7 +10,6 @@ import (
 
 	"github.com/absmach/magistrala"
 	mgclients "github.com/absmach/magistrala/pkg/clients"
-	mgoauth2 "github.com/absmach/magistrala/pkg/oauth2"
 	"github.com/absmach/magistrala/users"
 )
 
@@ -399,11 +398,10 @@ func (lm *loggingMiddleware) Identify(ctx context.Context, token string) (id str
 	return lm.svc.Identify(ctx, token)
 }
 
-func (lm *loggingMiddleware) OAuthCallback(ctx context.Context, state mgoauth2.State, client mgclients.Client) (token *magistrala.Token, err error) {
+func (lm *loggingMiddleware) OAuthCallback(ctx context.Context, client mgclients.Client) (token *magistrala.Token, err error) {
 	defer func(begin time.Time) {
 		args := []any{
 			slog.String("duration", time.Since(begin).String()),
-			slog.String("state", state.String()),
 			slog.String("user_id", client.ID),
 		}
 		if err != nil {
@@ -413,5 +411,5 @@ func (lm *loggingMiddleware) OAuthCallback(ctx context.Context, state mgoauth2.S
 		}
 		lm.logger.Info("OAuth callback completed successfully", args...)
 	}(time.Now())
-	return lm.svc.OAuthCallback(ctx, state, client)
+	return lm.svc.OAuthCallback(ctx, client)
 }

users/api/metrics.go

@@ -9,7 +9,6 @@ import (
 
 	"github.com/absmach/magistrala"
 	mgclients "github.com/absmach/magistrala/pkg/clients"
-	mgoauth2 "github.com/absmach/magistrala/pkg/oauth2"
 	"github.com/absmach/magistrala/users"
 	"github.com/go-kit/kit/metrics"
 )
@@ -193,11 +192,10 @@ func (ms *metricsMiddleware) Identify(ctx context.Context, token string) (string
 	return ms.svc.Identify(ctx, token)
 }
 
-func (ms *metricsMiddleware) OAuthCallback(ctx context.Context, state mgoauth2.State, client mgclients.Client) (*magistrala.Token, error) {
-	method := "oauth_callback_" + state.String()
+func (ms *metricsMiddleware) OAuthCallback(ctx context.Context, client mgclients.Client) (*magistrala.Token, error) {
 	defer func(begin time.Time) {
-		ms.counter.With("method", method).Add(1)
-		ms.latency.With("method", method).Observe(time.Since(begin).Seconds())
+		ms.counter.With("method", "oauth_callback").Add(1)
+		ms.latency.With("method", "oauth_callback").Observe(time.Since(begin).Seconds())
 	}(time.Now())
-	return ms.svc.OAuthCallback(ctx, state, client)
+	return ms.svc.OAuthCallback(ctx, client)
 }

users/clients.go

@@ -8,7 +8,6 @@ import (
 
 	"github.com/absmach/magistrala"
 	"github.com/absmach/magistrala/pkg/clients"
-	mgoauth2 "github.com/absmach/magistrala/pkg/oauth2"
 )
 
 // Service specifies an API that must be fullfiled by the domain service
@@ -77,5 +76,5 @@ type Service interface {
 
 	// OAuthCallback handles the callback from any supported OAuth provider.
 	// It processes the OAuth tokens and either signs in or signs up the user based on the provided state.
-	OAuthCallback(ctx context.Context, state mgoauth2.State, client clients.Client) (*magistrala.Token, error)
+	OAuthCallback(ctx context.Context, client clients.Client) (*magistrala.Token, error)
 }

users/events/events.go

@@ -414,14 +414,12 @@ func (spre sendPasswordResetEvent) Encode() (map[string]interface{}, error) {
 }
 
 type oauthCallbackEvent struct {
-	state    string
 	clientID string
 }
 
 func (oce oauthCallbackEvent) Encode() (map[string]interface{}, error) {
 	return map[string]interface{}{
 		"operation": oauthCallback,
-		"state":     oce.state,
 		"client_id": oce.clientID,
 	}, nil
 }

users/events/streams.go

@@ -10,7 +10,6 @@ import (
 	mgclients "github.com/absmach/magistrala/pkg/clients"
 	"github.com/absmach/magistrala/pkg/events"
 	"github.com/absmach/magistrala/pkg/events/store"
-	mgoauth2 "github.com/absmach/magistrala/pkg/oauth2"
 	"github.com/absmach/magistrala/users"
 )
 
@@ -297,14 +296,13 @@ func (es *eventStore) SendPasswordReset(ctx context.Context, host, email, user,
 	return es.Publish(ctx, event)
 }
 
-func (es *eventStore) OAuthCallback(ctx context.Context, state mgoauth2.State, client mgclients.Client) (*magistrala.Token, error) {
-	token, err := es.svc.OAuthCallback(ctx, state, client)
+func (es *eventStore) OAuthCallback(ctx context.Context, client mgclients.Client) (*magistrala.Token, error) {
+	token, err := es.svc.OAuthCallback(ctx, client)
 	if err != nil {
 		return token, err
 	}
 
 	event := oauthCallbackEvent{
-		state:    state.String(),
 		clientID: client.ID,
 	}
 

users/service.go

@@ -5,7 +5,6 @@ package users
 
 import (
 	"context"
-	"fmt"
 	"time"
 
 	"github.com/absmach/magistrala"
@@ -14,14 +13,12 @@ import (
 	"github.com/absmach/magistrala/pkg/errors"
 	repoerr "github.com/absmach/magistrala/pkg/errors/repository"
 	svcerr "github.com/absmach/magistrala/pkg/errors/service"
-	mgoauth2 "github.com/absmach/magistrala/pkg/oauth2"
 	"github.com/absmach/magistrala/users/postgres"
 	"golang.org/x/sync/errgroup"
 )
 
 var (
 	errIssueToken            = errors.New("failed to issue token")
-	errUserNotSignedUp       = errors.New("user not signed up")
 	errFailedPermissionsList = errors.New("failed to list permissions")
 	errRecoveryToken         = errors.New("failed to generate password recovery token")
 	errLoginDisableUser      = errors.New("failed to login in disabled user")
@@ -592,37 +589,32 @@ func (svc *service) authorize(ctx context.Context, subjType, subjKind, subj, per
 	return res.GetId(), nil
 }
 
-func (svc service) OAuthCallback(ctx context.Context, state mgoauth2.State, client mgclients.Client) (*magistrala.Token, error) {
-	switch state {
-	case mgoauth2.SignIn:
-		rclient, err := svc.clients.RetrieveByIdentity(ctx, client.Credentials.Identity)
-		if err != nil {
-			if errors.Contains(err, repoerr.ErrNotFound) {
-				return &magistrala.Token{}, errors.Wrap(svcerr.ErrNotFound, errUserNotSignedUp)
-			}
-			return &magistrala.Token{}, errors.Wrap(svcerr.ErrViewEntity, err)
-		}
-		claims := &magistrala.IssueReq{
-			UserId: rclient.ID,
-			Type:   uint32(auth.AccessKey),
-		}
-		return svc.auth.Issue(ctx, claims)
-	case mgoauth2.SignUp:
-		rclient, err := svc.RegisterClient(ctx, "", client)
-		if err != nil {
-			if errors.Contains(err, repoerr.ErrConflict) {
-				return &magistrala.Token{}, errors.Wrap(svcerr.ErrConflict, errors.New("user already exists"))
+func (svc service) OAuthCallback(ctx context.Context, client mgclients.Client) (*magistrala.Token, error) {
+	rclient, err := svc.clients.RetrieveByIdentity(ctx, client.Credentials.Identity)
+	if err != nil {
+		switch errors.Contains(err, repoerr.ErrNotFound) {
+		case true:
+			rclient, err = svc.RegisterClient(ctx, "", client)
+			if err != nil {
+				return &magistrala.Token{}, err
 			}
+		default:
 			return &magistrala.Token{}, err
 		}
-		claims := &magistrala.IssueReq{
-			UserId: rclient.ID,
-			Type:   uint32(auth.AccessKey),
+	}
+
+	if _, err = svc.authorize(ctx, auth.UserType, auth.UsersKind, rclient.ID, auth.MembershipPermission, auth.PlatformType, auth.MagistralaObject); err != nil {
+		if err := svc.addClientPolicy(ctx, rclient.ID, rclient.Role); err != nil {
+			return &magistrala.Token{}, err
 		}
-		return svc.auth.Issue(ctx, claims)
-	default:
-		return &magistrala.Token{}, fmt.Errorf("unknown state %s", state)
 	}
+
+	claims := &magistrala.IssueReq{
+		UserId: rclient.ID,
+		Type:   uint32(auth.AccessKey),
+	}
+
+	return svc.auth.Issue(ctx, claims)
 }
 
 func (svc service) Identify(ctx context.Context, token string) (string, error) {