Skip to content

Commit

Permalink
Merge pull request juju#17615 from kian99/revert-move-session-token-l…
Browse files Browse the repository at this point in the history
…ogin-provider

juju#17615

This PR partially reverts a change introduced in juju#17451, where CLI specific login providers were moved to an internal package. The problem with this change is that JAAS uses/imports the session token login provider so I've reverted the move.

Currently the only internal login provider is the `tryInOrderLoginProvider` kept in `cmd/internal/loginprovider`.

## Checklist

<!-- If an item is not applicable, use `~strikethrough~`. -->

- [x] Code style: imports ordered, good names, simple structure, etc
- [x] Comments saying why design decisions were made
- [ ] Go unit tests, with comments saying what you're testing
- [ ] [Integration tests](https://github.com/juju/juju/tree/main/tests), with comments saying what you're testing
- [ ] [doc.go](https://discourse.charmhub.io/t/readme-in-packages/451) added or updated in changed packages
  • Loading branch information
jujubot authored Jul 5, 2024
2 parents b51f6b5 + 5c1c6d0 commit acecfaa
Show file tree
Hide file tree
Showing 10 changed files with 18 additions and 27 deletions.
3 changes: 3 additions & 0 deletions api/export_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,9 @@ var (
SlideAddressToFront = slideAddressToFront
FacadeVersions = &facadeVersions

LoginDeviceAPICall = &loginDeviceAPICall
GetDeviceSessionTokenAPICall = &getDeviceSessionTokenAPICall
LoginWithSessionTokenAPICall = &loginWithSessionTokenAPICall
LoginWithClientCredentialsAPICall = &loginWithClientCredentialsAPICall
)

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@
// for login normally used by the CLI.
// These are contrasted with login providers defined elsewhere which may not
// require interactive login.
package loginprovider
package api

import (
"context"
Expand All @@ -14,7 +14,6 @@ import (

"github.com/juju/errors"

"github.com/juju/juju/api"
"github.com/juju/juju/api/base"
"github.com/juju/juju/rpc/params"
)
Expand Down Expand Up @@ -59,7 +58,7 @@ type sessionTokenLoginProvider struct {
//
// It authenticates as the entity using the specified session token.
// Subsequent requests on the state will act as that entity.
func (p *sessionTokenLoginProvider) Login(ctx context.Context, caller base.APICaller) (*api.LoginResultParams, error) {
func (p *sessionTokenLoginProvider) Login(ctx context.Context, caller base.APICaller) (*LoginResultParams, error) {
// First we try to log in using the session token we have.
result, err := p.login(ctx, caller)
if err == nil {
Expand Down Expand Up @@ -128,7 +127,7 @@ func (p *sessionTokenLoginProvider) initiateDeviceLogin(ctx context.Context, cal
return p.updateAccountDetailsFunc(sessionTokenResult.SessionToken)
}

func (p *sessionTokenLoginProvider) login(ctx context.Context, caller base.APICaller) (*api.LoginResultParams, error) {
func (p *sessionTokenLoginProvider) login(ctx context.Context, caller base.APICaller) (*LoginResultParams, error) {
var result params.LoginResult
request := struct {
SessionToken string `json:"session-token"`
Expand All @@ -141,5 +140,5 @@ func (p *sessionTokenLoginProvider) login(ctx context.Context, caller base.APICa
return nil, errors.Trace(err)
}

return api.NewLoginResultParams(result)
return NewLoginResultParams(result)
}
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
// Copyright 2024 Canonical Ltd.
// Licensed under the AGPLv3, see LICENCE file for details.

package loginprovider_test
package api_test

import (
"bytes"
Expand All @@ -14,7 +14,6 @@ import (

"github.com/juju/juju/api"
"github.com/juju/juju/api/base"
"github.com/juju/juju/cmd/internal/loginprovider"
jujutesting "github.com/juju/juju/juju/testing"
"github.com/juju/juju/rpc/params"
)
Expand All @@ -34,7 +33,7 @@ func (s *sessionTokenLoginProviderProviderSuite) Test(c *gc.C) {

var obtainedSessionToken string

s.PatchValue(loginprovider.LoginDeviceAPICall, func(_ base.APICaller, request interface{}, response interface{}) error {
s.PatchValue(api.LoginDeviceAPICall, func(_ base.APICaller, request interface{}, response interface{}) error {
lr := struct {
UserCode string `json:"user-code"`
VerificationURI string `json:"verification-uri"`
Expand All @@ -51,7 +50,7 @@ func (s *sessionTokenLoginProviderProviderSuite) Test(c *gc.C) {
return json.Unmarshal(data, response)
})

s.PatchValue(loginprovider.GetDeviceSessionTokenAPICall, func(_ base.APICaller, request interface{}, response interface{}) error {
s.PatchValue(api.GetDeviceSessionTokenAPICall, func(_ base.APICaller, request interface{}, response interface{}) error {
lr := struct {
SessionToken string `json:"session-token"`
}{
Expand All @@ -66,7 +65,7 @@ func (s *sessionTokenLoginProviderProviderSuite) Test(c *gc.C) {
return json.Unmarshal(data, response)
})

s.PatchValue(loginprovider.LoginWithSessionTokenAPICall, func(_ base.APICaller, request interface{}, response interface{}) error {
s.PatchValue(api.LoginWithSessionTokenAPICall, func(_ base.APICaller, request interface{}, response interface{}) error {
data, err := json.Marshal(request)
if err != nil {
return errors.Trace(err)
Expand Down Expand Up @@ -108,7 +107,7 @@ func (s *sessionTokenLoginProviderProviderSuite) Test(c *gc.C) {
ControllerUUID: info.ControllerUUID,
CACert: info.CACert,
}, api.DialOpts{
LoginProvider: loginprovider.NewSessionTokenLoginProvider(
LoginProvider: api.NewSessionTokenLoginProvider(
"expired-token",
&output,
func(sessionToken string) error {
Expand Down
File renamed without changes.
10 changes: 0 additions & 10 deletions cmd/internal/loginprovider/export_test.go

This file was deleted.

4 changes: 2 additions & 2 deletions cmd/internal/loginprovider/package_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -6,9 +6,9 @@ package loginprovider
import (
"testing"

coretesting "github.com/juju/juju/testing"
gc "gopkg.in/check.v1"
)

func TestPackage(t *testing.T) {
coretesting.MgoTestPackage(t)
gc.TestingT(t)
}
1 change: 1 addition & 0 deletions cmd/internal/loginprovider/tryinorderloginprovider.go
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
// Copyright 2024 Canonical Ltd.
// Licensed under the AGPLv3, see LICENCE file for details.

// Contains login providers required only by the Juju CLI.
package loginprovider

import (
Expand Down
2 changes: 1 addition & 1 deletion cmd/juju/controller/register.go
Original file line number Diff line number Diff line change
Expand Up @@ -280,7 +280,7 @@ func (c *registerCommand) publicControllerDetails(ctx *cmd.Context, host, contro
// user-pass or macaroons.
dialOpts.LoginProvider = loginprovider.NewTryInOrderLoginProvider(
loggo.GetLogger("juju.cmd.loginprovider"),
loginprovider.NewSessionTokenLoginProvider(
api.NewSessionTokenLoginProvider(
"",
ctx.Stderr,
func(sessionToken string) error {
Expand Down
2 changes: 1 addition & 1 deletion cmd/juju/user/login.go
Original file line number Diff line number Diff line change
Expand Up @@ -385,7 +385,7 @@ func (c *loginCommand) publicControllerLogin(

dialOpts.LoginProvider = loginprovider.NewTryInOrderLoginProvider(
loggo.GetLogger("juju.cmd.loginprovider"),
loginprovider.NewSessionTokenLoginProvider(
api.NewSessionTokenLoginProvider(
sessionToken,
ctx.Stderr,
func(sessionToken string) error {
Expand Down
3 changes: 1 addition & 2 deletions cmd/modelcmd/base.go
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,6 @@ import (
"github.com/juju/juju/api/client/modelmanager"
k8sproxy "github.com/juju/juju/caas/kubernetes/provider/proxy"
"github.com/juju/juju/cloud"
"github.com/juju/juju/cmd/internal/loginprovider"
"github.com/juju/juju/core/network"
"github.com/juju/juju/environs"
environscloudspec "github.com/juju/juju/environs/cloudspec"
Expand Down Expand Up @@ -593,7 +592,7 @@ func newAPIConnectionParams(
dialOpts.BakeryClient = bakery

if accountDetails.Type == jujuclient.OAuth2DeviceFlowAccountDetailsType {
dialOpts.LoginProvider = loginprovider.NewSessionTokenLoginProvider(
dialOpts.LoginProvider = api.NewSessionTokenLoginProvider(
accountDetails.SessionToken,
cmdOut,
func(sessionToken string) error {
Expand Down

0 comments on commit acecfaa

Please sign in to comment.