Css 6646/callback endpoint (#1170)

* feat(oauth login browser): implements /auth/login

As discussed on call, we integration test only the handler and avoid mocks to see the behaviour is
as expected. This is also true when we come to implements the callback logic and are required to
start the flow from scratch. The current test in auth_handler_test will be updated to cover the
entire flow when implementing /callback. As for the state todo, I need to see how to track the state
between requests and the TODO will be completed in the /callback PR.

6646

* pr comments

* pr comments

* feat(browser flow for dashboard): implements browser flow (without sessions)

This PR includes a small refactor to the admin device flow, such that it can share the same identity
update logic within the browser flow.

* feat(validation in auth handler): validates params given are correct and auth svc not nill

* Fix tests

* pr comments

* additional failure tests

* test fix

* Update refresh token

cmd/jimmsrv/main.go

@@ -152,6 +152,7 @@ func start(ctx context.Context, s *service.Service) error {
 			Scopes:             scopesParsed,
 			SessionTokenExpiry: sessionTokenExpiryDuration,
 		},
+		DashboardFinalRedirectURL: os.Getenv("JIMM_DASHBOARD_FINAL_REDIRECT_URL"),
 	})
 	if err != nil {
 		return err

docker-compose.yaml

@@ -78,6 +78,7 @@ services:
       JIMM_OAUTH_CLIENT_ID: "jimm-device"
       JIMM_OAUTH_CLIENT_SECRET: "SwjDofnbDzJDm9iyfUhEp67FfUFMY8L4"
       JIMM_OAUTH_SCOPES: "openid profile email" # Space separated list of scopes
+      JIMM_DASHBOARD_FINAL_REDIRECT_URL: "https://my-dashboard.com/final-callback" # Example URL
       JIMM_ACCESS_TOKEN_EXPIRY_DURATION: 1h
     volumes:
       - ./:/jimm/

internal/auth/oauth2.go

@@ -7,6 +7,7 @@ import (
 	"encoding/base64"
 	stderrors "errors"
 	"net/mail"
+	"strings"
 	"time"
 
 	"github.com/coreos/go-oidc/v3/oidc"
@@ -16,6 +17,7 @@ import (
 	"go.uber.org/zap"
 	"golang.org/x/oauth2"
 
+	"github.com/canonical/jimm/internal/dbmodel"
 	"github.com/canonical/jimm/internal/errors"
 )
 
@@ -27,6 +29,15 @@ type AuthenticationService struct {
 	provider *oidc.Provider
 	// sessionTokenExpiry holds the expiry time for JIMM minted session tokens (JWTs).
 	sessionTokenExpiry time.Duration
+
+	db IdentityStore
+}
+
+// Identity store holds the necessary methods to get and update an identity
+// within JIMM's store.
+type IdentityStore interface {
+	GetIdentity(ctx context.Context, u *dbmodel.Identity) error
+	UpdateIdentity(ctx context.Context, u *dbmodel.Identity) error
 }
 
 // AuthenticationServiceParams holds the parameters to initialise
@@ -48,6 +59,11 @@ type AuthenticationServiceParams struct {
 	// codes into access tokens (and id tokens), for JIMM, this is expected
 	// to be the servers own callback endpoint registered under /auth/callback.
 	RedirectURL string
+
+	// Store holds the identity store used by the authentication service
+	// to fetch and update identities. I.e., their access tokens, refresh tokens,
+	// display name, etc.
+	Store IdentityStore
 }
 
 // NewAuthenticationService returns a new authentication service for handling
@@ -71,6 +87,7 @@ func NewAuthenticationService(ctx context.Context, params AuthenticationServiceP
 			RedirectURL:  params.RedirectURL,
 		},
 		sessionTokenExpiry: params.SessionTokenExpiry,
+		db:                 params.Store,
 	}, nil
 }
 
@@ -86,6 +103,27 @@ func (as *AuthenticationService) AuthCodeURL() string {
 	return as.oauthConfig.AuthCodeURL("")
 }
 
+// Exchange exchanges an authorisation code for an access token.
+//
+// TODO(ale8k): How to test this? A callback has to be made and it needs to be valid,
+// this may need some thought as to whether its actually worth testing or are we
+// just testing the library. The handler test essentially covers this so perhaps
+// its ok to leave it as is?
+func (as *AuthenticationService) Exchange(ctx context.Context, code string) (*oauth2.Token, error) {
+	const op = errors.Op("auth.AuthenticationService.Exchange")
+
+	t, err := as.oauthConfig.Exchange(
+		ctx,
+		code,
+		oauth2.SetAuthURLParam("client_secret", as.oauthConfig.ClientSecret),
+	)
+	if err != nil {
+		return nil, errors.E(op, err, "authorisation code exchange failed")
+	}
+
+	return t, nil
+}
+
 // Device initiates a device flow login and is step ONE of TWO.
 //
 // This is done via retrieving a:
@@ -203,6 +241,34 @@ func (as *AuthenticationService) VerifySessionToken(token string, secretKey stri
 	return VerifySessionToken(token, secretKey)
 }
 
+// UpdateIdentity updates the database with the display name and access token set for the user.
+// And, if present, a refresh token.
+func (as *AuthenticationService) UpdateIdentity(ctx context.Context, email string, token *oauth2.Token) error {
+	db := as.db
+	u := &dbmodel.Identity{
+		Name: email,
+	}
+	// TODO(babakks): If user does not exist, we will create one with an empty
+	// display name (which we shouldn't). So it would be better to fetch
+	// and then create. At the moment, GetUser is used for both create and fetch,
+	// this should be changed and split apart so it is intentional what entities
+	// we are creating or fetching.
+	if err := db.GetIdentity(ctx, u); err != nil {
+		return err
+	}
+	// Check if user has a display name, if not, set one
+	if u.DisplayName == "" {
+		u.DisplayName = strings.Split(email, "@")[0]
+	}
+	u.AccessToken = token.AccessToken
+	u.RefreshToken = token.RefreshToken
+	if err := db.UpdateIdentity(ctx, u); err != nil {
+		return err
+	}
+
+	return nil
+}
+
 // VerifySessionToken symmetrically verifies the validty of the signature on the
 // access token JWT, returning the parsed token.
 //

internal/auth/oauth2_test.go

@@ -14,23 +14,31 @@ import (
 	"time"
 
 	"github.com/canonical/jimm/internal/auth"
+	"github.com/canonical/jimm/internal/db"
+	"github.com/canonical/jimm/internal/dbmodel"
 	"github.com/canonical/jimm/internal/jimmtest"
 	"github.com/coreos/go-oidc/v3/oidc"
 	qt "github.com/frankban/quicktest"
 )
 
-func setupTestAuthSvc(ctx context.Context, c *qt.C, expiry time.Duration) *auth.AuthenticationService {
+func setupTestAuthSvc(ctx context.Context, c *qt.C, expiry time.Duration) (*auth.AuthenticationService, *db.Database) {
+	db := &db.Database{
+		DB: jimmtest.PostgresDB(c, func() time.Time { return time.Now() }),
+	}
+	c.Assert(db.Migrate(ctx, false), qt.IsNil)
+
 	authSvc, err := auth.NewAuthenticationService(ctx, auth.AuthenticationServiceParams{
 		IssuerURL:          "http://localhost:8082/realms/jimm",
 		ClientID:           "jimm-device",
 		ClientSecret:       "SwjDofnbDzJDm9iyfUhEp67FfUFMY8L4",
 		Scopes:             []string{oidc.ScopeOpenID, "profile", "email"},
 		SessionTokenExpiry: expiry,
 		RedirectURL:        "http://localhost:8080/auth/callback",
+		Store:              db,
 	})
 	c.Assert(err, qt.IsNil)
 
-	return authSvc
+	return authSvc, db
 }
 
 // This test requires the local docker compose to be running and keycloak
@@ -41,7 +49,7 @@ func TestAuthCodeURL(t *testing.T) {
 	c := qt.New(t)
 	ctx := context.Background()
 
-	authSvc := setupTestAuthSvc(ctx, c, time.Hour)
+	authSvc, _ := setupTestAuthSvc(ctx, c, time.Hour)
 
 	url := authSvc.AuthCodeURL()
 	c.Assert(
@@ -67,7 +75,7 @@ func TestDevice(t *testing.T) {
 
 	ctx := context.Background()
 
-	authSvc := setupTestAuthSvc(ctx, c, time.Hour)
+	authSvc, db := setupTestAuthSvc(ctx, c, time.Hour)
 
 	res, err := authSvc.Device(ctx)
 	c.Assert(err, qt.IsNil)
@@ -138,6 +146,17 @@ func TestDevice(t *testing.T) {
 	email, err := authSvc.Email(idToken)
 	c.Assert(err, qt.IsNil)
 	c.Assert(email, qt.Equals, u.Email)
+
+	// Update the identity
+	err = authSvc.UpdateIdentity(ctx, email, token)
+	c.Assert(err, qt.IsNil)
+
+	updatedUser := &dbmodel.Identity{
+		Name: u.Email,
+	}
+	c.Assert(db.GetIdentity(ctx, updatedUser), qt.IsNil)
+	c.Assert(updatedUser.AccessToken, qt.Not(qt.Equals), "")
+	c.Assert(updatedUser.RefreshToken, qt.Not(qt.Equals), "")
 }
 
 // TestSessionTokens tests both the minting and validation of JIMM
@@ -147,7 +166,7 @@ func TestSessionTokens(t *testing.T) {
 
 	ctx := context.Background()
 
-	authSvc := setupTestAuthSvc(ctx, c, time.Hour)
+	authSvc, _ := setupTestAuthSvc(ctx, c, time.Hour)
 
 	secretKey := "secret-key"
 	token, err := authSvc.MintSessionToken("[email protected]", secretKey)
@@ -164,7 +183,7 @@ func TestSessionTokenRejectsWrongSecretKey(t *testing.T) {
 
 	ctx := context.Background()
 
-	authSvc := setupTestAuthSvc(ctx, c, time.Hour)
+	authSvc, _ := setupTestAuthSvc(ctx, c, time.Hour)
 
 	secretKey := "secret-key"
 	token, err := authSvc.MintSessionToken("[email protected]", secretKey)
@@ -181,7 +200,7 @@ func TestSessionTokenRejectsExpiredToken(t *testing.T) {
 	ctx := context.Background()
 
 	noDuration := time.Duration(0)
-	authSvc := setupTestAuthSvc(ctx, c, noDuration)
+	authSvc, _ := setupTestAuthSvc(ctx, c, noDuration)
 
 	secretKey := "secret-key"
 	token, err := authSvc.MintSessionToken("[email protected]", secretKey)
@@ -197,7 +216,7 @@ func TestSessionTokenValidatesEmail(t *testing.T) {
 
 	ctx := context.Background()
 
-	authSvc := setupTestAuthSvc(ctx, c, time.Hour)
+	authSvc, _ := setupTestAuthSvc(ctx, c, time.Hour)
 
 	secretKey := "secret-key"
 	token, err := authSvc.MintSessionToken("", secretKey)

internal/cmdtest/jimmsuite.go

@@ -91,6 +91,7 @@ func (s *JimmCmdSuite) SetUpTest(c *gc.C) {
 			Scopes:             []string{oidc.ScopeOpenID, "profile", "email"},
 			SessionTokenExpiry: time.Duration(time.Hour),
 		},
+		DashboardFinalRedirectURL: "<no dashboard needed for this test>",
 	}
 
 	srv, err := service.NewService(ctx, s.Params)

internal/dbmodel/identity.go

@@ -39,6 +39,11 @@ type Identity struct {
 	// from the browser or device flow, and as such is updated on every successful
 	// login.
 	AccessToken string
+
+	// RefreshToken is an OAuth2.0 refresh token for this identity, it may have come
+	// from the browser or device flow, and as such is updated on every successful
+	// login.
+	RefreshToken string
 }
 
 // Tag returns a names.Tag for the identity.

internal/dbmodel/sql/postgres/1_6.sql

@@ -1,6 +1,7 @@
 -- 1_6.sql is a migration that adds access tokens to the user table
 -- and is a migration that renames `user` to `identity`.
 ALTER TABLE users ADD COLUMN access_token TEXT;
+ALTER TABLE users ADD COLUMN refresh_token TEXT;
 
 -- Note that we don't need to rename underlying indexes/constraints. As Postgres
 -- docs states:

internal/jimm/jimm.go

@@ -165,6 +165,10 @@ type OAuthAuthenticator interface {
 	// The subject of the token contains the user's email and can be used
 	// for user object creation.
 	VerifySessionToken(token string, secretKey string) (jwt.Token, error)
+
+	// UpdateIdentity updates the database with the display name and access token set for the user.
+	// And, if present, a refresh token.
+	UpdateIdentity(ctx context.Context, email string, token *oauth2.Token) error
 }
 
 type permission struct {

internal/jimm/user_test.go

@@ -108,21 +108,22 @@ func TestGetOpenFGAUser(t *testing.T) {
 	client, _, _, err := jimmtest.SetupTestOFGAClient(c.Name())
 	c.Assert(err, qt.IsNil)
 
+	db := &db.Database{
+		DB: jimmtest.PostgresDB(c, func() time.Time { return time.Now() }),
+	}
 	// TODO(ale8k): Mock this
 	authSvc, err := auth.NewAuthenticationService(ctx, auth.AuthenticationServiceParams{
 		IssuerURL:          "http://localhost:8082/realms/jimm",
 		ClientID:           "jimm-device",
 		Scopes:             []string{"openid", "profile", "email"},
 		SessionTokenExpiry: time.Hour,
+		Store:              db,
 	})
 	c.Assert(err, qt.IsNil)
 
-	now := time.Now().UTC().Round(time.Millisecond)
 	j := &jimm.JIMM{
-		UUID: "test",
-		Database: db.Database{
-			DB: jimmtest.PostgresDB(c, func() time.Time { return now }),
-		},
+		UUID:               "test",
+		Database:           *db,
 		OAuthAuthenticator: authSvc,
 		OpenFGAClient:      client,
 	}