Skip to content
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.

Sign up for GitHub

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

feat!: customizable client scopes in oidc_impl package #1052

Closed
wants to merge 27 commits into from
+37 −10
Closed

feat!: customizable client scopes in oidc_impl package #1052

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
27 commits
Select commit Hold shift + click to select a range
71c1473
wip: test a method of adding scopes
kuannie1 Apr 12, 2024
12cc78e
fix: try logging a bit more
kuannie1 Apr 12, 2024
ff65765
try just hardcoding one
kuannie1 Apr 12, 2024
0a8daef
add another debug statement for re-authenticating
kuannie1 Apr 12, 2024
6a811e6
try another delimiter
kuannie1 Apr 12, 2024
8a4e222
add another debug statement
kuannie1 Apr 12, 2024
c1e8bf1
try setting offline_access
kuannie1 Apr 12, 2024
8933f6a
try to include scopes
kuannie1 Apr 12, 2024
2329788
go back to +
kuannie1 Apr 12, 2024
0ce068b
fix: exclude email & groups for now
kuannie1 Apr 12, 2024
5f00c36
try adding some other scopes
kuannie1 Apr 13, 2024
8a6f8d4
empty initial scopes--add scopes if needed
kuannie1 Apr 16, 2024
2daaa57
unneeded debug statement
kuannie1 Apr 16, 2024
4f2e1ce
try original logic adding scopes
kuannie1 Apr 16, 2024
5fe29e8
Merge branch 'aku/set-scopes-if-needed' of github.com:chanzuckerberg/…
kuannie1 Apr 16, 2024
7d477e1
try this part again
kuannie1 Apr 16, 2024
0f78372
try this again
kuannie1 Apr 16, 2024
8f33632
add helper comment
kuannie1 Apr 16, 2024
f0722ed
don't need this
kuannie1 Apr 16, 2024
03f9ec1
don't need this
kuannie1 Apr 16, 2024
0dc6930
re-add space
kuannie1 Apr 16, 2024
03b9fee
re-add space
kuannie1 Apr 16, 2024
538dc94
Merge branch 'main' of github.com:chanzuckerberg/go-misc into aku/set…
kuannie1 Apr 16, 2024
19d1b5d
Merge branch 'main' of github.com:chanzuckerberg/go-misc into aku/set…
kuannie1 Apr 16, 2024
8ba7c51
small correction to debug statement
kuannie1 Apr 16, 2024
0354306
Merge branch 'main' into aku/set-scopes-if-needed
kuannie1 May 23, 2024
615e5d6
fix: add client secret
kuannie1 Sep 26, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
32 changes: 22 additions & 10 deletions oidc_cli/oidc_impl/client/client.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@ import (
"fmt"
"io"
"os"
"strings"
"time"

"github.com/coreos/go-oidc"
Expand Down Expand Up @@ -51,12 +52,7 @@ func NewClient(ctx context.Context, config *Config, clientOptions ...Option) (*C
ClientID: config.ClientID,
RedirectURL: fmt.Sprintf("http://localhost:%d", server.GetBoundPort()),
Endpoint: provider.Endpoint(),
Scopes: []string{
oidc.ScopeOpenID,
oidc.ScopeOfflineAccess,
"email",
"groups",
Comment on lines -55 to -58
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Will we always have to add all these scopes manually now? If so, would it be worth adding a function AddDefaultScopes that adds all of these?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@hspitzley-czi That was my initial idea, but I'd understand if that's not ideal!

Do you mean adding a function like this to oidc_cli/oidc_impl/client/config_options.go?

var AddDefaultScopes = func(scope string) Option {
	return func(c *Client) {
		c.oauthConfig.Scopes = []string{"openid", "groups", "email", "offline_access"}
	}
}

Or like, adding all the default scopes if none are provided?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah like the AddDefaultScopes you defined here! Adding default scopes if none are provided would be nice too if possible, that way it's backwards compatible

},
Scopes: []string{}, // add through the AddScope clientOption
}

oidcConfig := &oidc.Config{
Expand Down Expand Up @@ -184,15 +180,31 @@ func (c *Client) ValidateState(ourState []byte, otherState []byte) error {
}
return nil
}
func format_scopes(ctx context.Context, scopes []string) string {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

For consistency, let's user camelcase, and rename to formatScopes().

// space-separated string:
// https://www.oauth.com/oauth2-servers/server-side-apps/authorization-code/#:~:text=with%20the%20service.-,scope,(optional),-Include%20one%20or

return strings.Join(scopes, "+")
}

// Exchange will exchange a token
func (c *Client) Exchange(ctx context.Context, code string, codeVerifier string) (*oauth2.Token, error) {
params := []oauth2.AuthCodeOption{oauth2.SetAuthURLParam("grant_type", "authorization_code"),
oauth2.SetAuthURLParam("code_verifier", codeVerifier),
oauth2.SetAuthURLParam("client_id", c.oauthConfig.ClientID),
}

if len(c.oauthConfig.Scopes) != 0 {
scope_str := format_scopes(ctx, c.oauthConfig.Scopes)
params = append(params, oauth2.SetAuthURLParam("scopes", scope_str))
logrus.Debugf("oauth scopes: %s", scope_str)
} else {
logrus.Debug("no scopes set")
}
token, err := c.oauthConfig.Exchange(
ctx,
code,
oauth2.SetAuthURLParam("grant_type", "authorization_code"),
oauth2.SetAuthURLParam("code_verifier", codeVerifier),
oauth2.SetAuthURLParam("client_id", c.oauthConfig.ClientID),
params...,
)
return token, errors.Wrap(err, "failed to exchange oauth token")
}
Expand Down Expand Up @@ -221,7 +233,7 @@ func (c *Client) Authenticate(ctx context.Context) (*Token, error) {
if err != nil {
return nil, err
}

logrus.Debugf("authenticate scopes: %+v", c.oauthConfig.Scopes)
c.server.Start(ctx, c, oauthMaterial)
fmt.Fprintf(os.Stderr, "Opening browser in order to authenticate with Okta, hold on a brief second...\n")
time.Sleep(2 * time.Second)
Expand Down
15 changes: 15 additions & 0 deletions oidc_cli/oidc_impl/client/config_options.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -23,3 +23,18 @@ var SetOauth2AuthStyle = func(authStyle oauth2.AuthStyle) Option {
c.oauthConfig.Endpoint.AuthStyle = authStyle
}
}

// This Helper helps you customize the scopes you're sending. It will format the list of strings
//
// example: https://www.oauth.com/oauth2-servers/server-side-apps/authorization-code/#:~:text=The%20authorization%20URL%20is%20usually%20in%20a%20format%20such%20as%3A
var AddScope = func(scope string) Option {
return func(c *Client) {
c.oauthConfig.Scopes = append(c.oauthConfig.Scopes, scope)
}
}

var AddClientSecret = func(secret string) Option {
return func(c *Client) {
c.oauthConfig.ClientSecret = secret
}
}
Loading