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.

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

Add scope support to access tokens #185

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

thinkst-marco
Copy link

- What I did

Add support for token scopes. Access Tokens created in the web interface can take a single scope value but this isn't available in hub-tool. This change lets the user create tokens with a specific scope.

- How I did it

Two user-facing changes:

  1. Added a --scope parameter to token create.
  2. Added a SCOPE column to token ls

- How to verify it

The following calls add tokens with the specified scopes:

$ hub-tool token create --scope public_read --format json | jq -c '.Scopes'
[ "repo:public_read" ]
$ hub-tool token create --scope read --format json | jq -c '.Scopes'
[ "repo:read" ]
$ hub-tool token create --description test-cli-scope-5 --scope write --format json | jq -c '.Scopes'
[ "repo:write" ]
$ hub-tool token create --description test-cli-scope-5 --scope admin --format json | jq -c '.Scopes'
[ "repo:admin" ]

Showing the added column in token ls:

$ hub-tool token ls
DESCRIPTION         UUID                                    LAST USED         CREATED          ACTIVE    SCOPE
[...]

- Description for the changelog

Add scope support to access tokens

- A picture of a cute animal (not mandatory)

Signed-off-by: Marco Slaviero <marco@thinkst.com>
@zdhamasha
Copy link

can anyone merge this

}

// CreateToken creates a Personal Access Token and returns the token field only once
func (c *Client) CreateToken(description string) (*Token, error) {
data, err := json.Marshal(hubTokenRequest{Description: description})
func (c *Client) CreateToken(description string, scope string) (*Token, error) {
Copy link
Member

Choose a reason for hiding this comment

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

nit:

Suggested change
func (c *Client) CreateToken(description string, scope string) (*Token, error) {
func (c *Client) CreateToken(description, scope string) (*Token, error) {

Comment on lines +56 to +58
scopes := []string{scope}
scopes[0] = "repo:" + scope
tokenRequest.Scopes = scopes
Copy link
Member

Choose a reason for hiding this comment

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

Am I missing something? Why can’t it be

Suggested change
scopes := []string{scope}
scopes[0] = "repo:" + scope
tokenRequest.Scopes = scopes
tokenRequest.Scopes = []string{"repo:" + scope}

?

if len(t.Scopes) == 0 {
return "", 0
}
return t.Scopes[0][5:], len(t.Scopes[0]) - 5
Copy link
Member

Choose a reason for hiding this comment

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

I guess 5 is because it’s the length of "repo:", could we factor out the string and use len()?

Comment on lines +48 to +51
"admin",
"write",
"read",
"public_read":
Copy link
Member

Choose a reason for hiding this comment

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

I would extract these into a []string and use it with strings.Join to build the description for the flag l.78

@jaythamke
Copy link

Is there new update here? Or code review required?
regards,
Jayesh

@converge converge mentioned this pull request Apr 9, 2022
@jaythamke
Copy link

Hi @converge, I observed that you mentioned this PR in your fix #203 , but we do not find the mention in your PR. Could you please confirm if the changes in this PR are included in your PR?

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.

4 participants