-
Notifications
You must be signed in to change notification settings - Fork 100
feat: add WithPrompt option for AuthCodeURL #585
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Changes from 3 commits
d411fb3
e52ff6b
4e11b8e
9a8a044
e1d9ce1
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -22,6 +22,7 @@ import ( | |
| "github.com/AzureAD/microsoft-authentication-library-for-go/apps/internal/oauth/ops/accesstokens" | ||
| "github.com/AzureAD/microsoft-authentication-library-for-go/apps/internal/oauth/ops/authority" | ||
| "github.com/AzureAD/microsoft-authentication-library-for-go/apps/internal/oauth/ops/wstrust" | ||
| "github.com/AzureAD/microsoft-authentication-library-for-go/apps/internal/shared" | ||
| "github.com/kylelemons/godebug/pretty" | ||
| ) | ||
|
|
||
|
|
@@ -43,16 +44,16 @@ func fakeBrowserOpenURL(authURL string) error { | |
| if m := q.Get("code_challenge_method"); m != "S256" { | ||
| return fmt.Errorf("unexpected code_challenge_method '%s'", m) | ||
| } | ||
| if q.Get("prompt") == "" { | ||
| return errors.New("missing query param 'prompt") | ||
| } | ||
| // if q.Get("prompt") == "" { | ||
| // return errors.New("missing query param 'prompt") | ||
|
||
| // } | ||
| state := q.Get("state") | ||
| if state == "" { | ||
| return errors.New("missing query param 'state'") | ||
| } | ||
| redirect := q.Get("redirect_uri") | ||
| if redirect == "" { | ||
| return errors.New("missing query param 'redirect_uri'") | ||
| return errors.New(" 'redirect_uri'") | ||
szogoon marked this conversation as resolved.
Outdated
Show resolved
Hide resolved
|
||
| } | ||
| // now send the info to our local redirect server | ||
| resp, err := http.DefaultClient.Get(redirect + fmt.Sprintf("/?state=%s&code=fake_auth_code", state)) | ||
|
|
@@ -935,6 +936,78 @@ func TestWithDomainHint(t *testing.T) { | |
| } | ||
| } | ||
|
|
||
| func TestWithPrompt(t *testing.T) { | ||
| prompt := shared.PromptCreate | ||
| client, err := New("client-id") | ||
| if err != nil { | ||
| t.Fatal(err) | ||
| } | ||
| client.base.Token.AccessTokens = &fake.AccessTokens{} | ||
| client.base.Token.Authority = &fake.Authority{} | ||
| client.base.Token.Resolver = &fake.ResolveEndpoints{} | ||
| for _, expectPrompt := range []bool{true, false} { | ||
| t.Run(fmt.Sprint(expectPrompt), func(t *testing.T) { | ||
| called := false | ||
| validate := func(v url.Values) error { | ||
| if !v.Has("prompt") { | ||
| if !expectPrompt { | ||
| return nil | ||
| } | ||
| return errors.New("expected a prompt") | ||
| } else if !expectPrompt { | ||
| return fmt.Errorf("expected no prompt, got %v", v["prompt"][0]) | ||
| } | ||
|
|
||
| if actual := v["prompt"]; len(actual) != 1 || actual[0] != prompt.String() { | ||
| err = fmt.Errorf(`unexpected prompt "%v"`, actual[0]) | ||
| } | ||
| return err | ||
| } | ||
| browserOpenURL := func(authURL string) error { | ||
| called = true | ||
| parsed, err := url.Parse(authURL) | ||
| if err != nil { | ||
| return err | ||
| } | ||
| query, err := url.ParseQuery(parsed.RawQuery) | ||
| if err != nil { | ||
| return err | ||
| } | ||
| if err = validate(query); err != nil { | ||
| t.Fatal(err) | ||
| return err | ||
| } | ||
| // this helper validates the other params and completes the redirect | ||
| return fakeBrowserOpenURL(authURL) | ||
| } | ||
| acquireOpts := []AcquireInteractiveOption{WithOpenURL(browserOpenURL)} | ||
| var urlOpts []AuthCodeURLOption | ||
| if expectPrompt { | ||
| acquireOpts = append(acquireOpts, WithPrompt(prompt)) | ||
| urlOpts = append(urlOpts, WithPrompt(prompt)) | ||
| } | ||
| _, err = client.AcquireTokenInteractive(context.Background(), tokenScope, acquireOpts...) | ||
| if err != nil { | ||
| t.Fatal(err) | ||
| } | ||
| if !called { | ||
| t.Fatal("browserOpenURL wasn't called") | ||
| } | ||
| u, err := client.AuthCodeURL(context.Background(), "id", "https://localhost", tokenScope, urlOpts...) | ||
| if err == nil { | ||
| var parsed *url.URL | ||
| parsed, err = url.Parse(u) | ||
| if err == nil { | ||
| err = validate(parsed.Query()) | ||
| } | ||
| } | ||
| if err != nil { | ||
| t.Fatal(err) | ||
| } | ||
| }) | ||
| } | ||
| } | ||
|
|
||
| func TestWithAuthenticationScheme(t *testing.T) { | ||
| clientInfo := base64.RawStdEncoding.EncodeToString([]byte(`{"uid":"uid","utid":"utid"}`)) | ||
| lmo, tenant := "login.microsoftonline.com", "tenant" | ||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We’re now changing the default from
"select_account"to an empty string"".Will need to test this a bit more to ensure everything behaves as expected.
Uh oh!
There was an error while loading. Please reload this page.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I was not aware that the "select_account" was the default value. That seems to be debatable choice in the first place. The default value on the protocol level is empty or completely absent.
In any case, changing the default from "select_account" to empty "" shall not cause catastrophic result because, again that was the default value in the specs and presumably most other libraries do that all the time. The perceived behavior will change, though, as the account selector will not pop up as often; but I think that shall be a welcome change.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Should this be tested live ?