fix: amr claim should contain provider_id for sso method (#2033)

## What kind of change does this PR introduce?
* When a SSO user has MFA enabled and signs in, the `sso/saml` amr claim
does not contain the provider field
* Usage of sort.Reverse was previously incorrect and the comments made
did not match what was expected - sort.Reverse needs to be chained with
sort.Sort and doesn't sort the array if used on it's own (see
[docs](https://pkg.go.dev/sort#Reverse))
* Opted to remove the sorting structs because we can just do it with
sort.Slice which is simpler

Author: kangmingtay

internal/models/sessions.go

@@ -71,22 +71,6 @@ type AMREntry struct {
 	Provider  string `json:"provider,omitempty"`
 }
 
-type sortAMREntries struct {
-	Array []AMREntry
-}
-
-func (s sortAMREntries) Len() int {
-	return len(s.Array)
-}
-
-func (s sortAMREntries) Less(i, j int) bool {
-	return s.Array[i].Timestamp < s.Array[j].Timestamp
-}
-
-func (s sortAMREntries) Swap(i, j int) {
-	s.Array[j], s.Array[i] = s.Array[i], s.Array[j]
-}
-
 type Session struct {
 	ID     uuid.UUID `json:"-" db:"id"`
 	UserID uuid.UUID `json:"user_id" db:"user_id"`
@@ -328,41 +312,27 @@ func (s *Session) CalculateAALAndAMR(user *User) (aal AuthenticatorAssuranceLeve
 		if claim.IsAAL2Claim() {
 			aal = AAL2
 		}
-		amr = append(amr, AMREntry{Method: claim.GetAuthenticationMethod(), Timestamp: claim.UpdatedAt.Unix()})
-	}
-
-	// makes sure that the AMR claims are always ordered most-recent first
-
-	// sort in ascending order
-	sort.Sort(sortAMREntries{
-		Array: amr,
-	})
-
-	// now reverse for descending order
-	_ = sort.Reverse(sortAMREntries{
-		Array: amr,
-	})
-
-	lastIndex := len(amr) - 1
-
-	if lastIndex > -1 && amr[lastIndex].Method == SSOSAML.String() {
-		// initial AMR claim is from sso/saml, we need to add information
-		// about the provider that was used for the authentication
-		identities := user.Identities
-
-		if len(identities) == 1 {
-			identity := identities[0]
-
-			if identity.IsForSSOProvider() {
-				amr[lastIndex].Provider = strings.TrimPrefix(identity.Provider, "sso:")
+		entry := AMREntry{Method: claim.GetAuthenticationMethod(), Timestamp: claim.UpdatedAt.Unix()}
+		if entry.Method == SSOSAML.String() {
+			// SSO users should only have one identity since they are excluded from account linking
+			// These checks act as a safeguard in the event future changes break this assumption.
+			identities := user.Identities
+			if len(identities) == 1 {
+				identity := identities[0]
+				if identity.IsForSSOProvider() {
+					entry.Provider = strings.TrimPrefix(identity.Provider, "sso:")
+				}
 			}
 		}
+		amr = append(amr, entry)
 
-		// otherwise we can't identify that this user account has only
-		// one SSO identity, so we are not encoding the provider at
-		// this time
 	}
 
+	// makes sure that the AMR claims are always ordered most-recent first
+	sort.Slice(amr, func(i, j int) bool {
+		return amr[i].Timestamp > amr[j].Timestamp
+	})
+
 	return aal, amr, nil
 }
 

internal/models/sessions_test.go

@@ -1,6 +1,7 @@
 package models
 
 import (
+	"strings"
 	"testing"
 	"time"
 
@@ -79,6 +80,13 @@ func (ts *SessionsTestSuite) TestCalculateAALAndAMR() {
 
 	session = ts.AddClaimAndReloadSession(session, TOTPSignIn)
 
+	identity, err := NewIdentity(u, "sso:95d4a792-4a2a-4523-ae63-bae0631de554", map[string]interface{}{
+		"sub": u.GetEmail(),
+	})
+	require.NoError(ts.T(), err)
+	require.NoError(ts.T(), ts.db.Create(identity))
+	u.Identities = append(u.Identities, *identity)
+
 	session = ts.AddClaimAndReloadSession(session, SSOSAML)
 
 	aal, amr, err := session.CalculateAALAndAMR(u)
@@ -97,7 +105,7 @@ func (ts *SessionsTestSuite) TestCalculateAALAndAMR() {
 
 	for _, claim := range amr {
 		if claim.Method == SSOSAML.String() {
-			require.NotNil(ts.T(), claim.Provider)
+			require.Equal(ts.T(), strings.TrimPrefix(identity.Provider, "sso:"), claim.Provider)
 		}
 	}
 	require.True(ts.T(), found)