Skip to content

Allow configuring scopes and extraUserInfo#98

Draft
joscha-alisch wants to merge 1 commit into
go-pkgz:masterfrom
joscha-alisch:master
Draft

Allow configuring scopes and extraUserInfo#98
joscha-alisch wants to merge 1 commit into
go-pkgz:masterfrom
joscha-alisch:master

Conversation

@joscha-alisch

Copy link
Copy Markdown

This adds a new method to add providers that allows to configure additional scopes and a hook that is called with the authenticated HTTP client. This is a draft to resolve issue #97 .

Usage is like this for the GitHub provider for example, which would fetch the orgs/teams a user is in and attach them to the user.Token.

service.AddProviderWithOptions("github", "cid", "csecret", []string{"read:org"}, func(c *http.Client, u token.User) token.User {
    gh := github.NewClient(c)
    t, _, _ := gh.Teams.ListUserTeams(context.Background(), &github.ListOptions{})
    
    var teams []string
    for _, team := range t {
	    teams = append(teams, fmt.Sprintf("%s:%s", team.Organization.GetLogin(), team.GetName()))
    }
    
    u.SetSliceAttr("teams", teams)
    return u
})

@joscha-alisch joscha-alisch requested a review from umputun as a code owner October 6, 2021 10:23
@joscha-alisch joscha-alisch marked this pull request as draft October 6, 2021 10:23
@coveralls

Copy link
Copy Markdown

Pull Request Test Coverage Report for Build 1311457085

  • 4 of 17 (23.53%) changed or added relevant lines in 1 file are covered.
  • No unchanged relevant lines lost coverage.
  • Overall coverage decreased (-4.8%) to 90.734%

Changes Missing Coverage Covered Lines Changed/Added Lines %
auth.go 4 17 23.53%
Totals Coverage Status
Change from base Build 1201515370: -4.8%
Covered Lines: 235
Relevant Lines: 259

💛 - Coveralls

@umputun

umputun commented Oct 11, 2021

Copy link
Copy Markdown
Member

it would be nice to cover the change with tests. almost 5% coverage decrease by itself is not such a problem, however not having any tests for extra functionality - is a problem.

@joscha-alisch

Copy link
Copy Markdown
Author

Certainly :) this was not intended as a ready-to-merge PR, but rather as a first iteration to get feedback on the direction. Hence the draft PR.

I'll go ahead and add tests.

@umputun

umputun commented Oct 30, 2021

Copy link
Copy Markdown
Member

@joscha-alisch I'm not sure if you expect me to review the PR as it is marked as draft

@joscha-alisch

joscha-alisch commented Oct 31, 2021

Copy link
Copy Markdown
Author

No, i Just didn't have time to add the tests yet. Will let you know when it's ready :)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants