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

Jump to bottom

initial version of group access token #50

Draft
wants to merge 2 commits into
base: main
Choose a base branch
from
Draft

initial version of group access token #50

wants to merge 2 commits into from

Conversation

M0roSan
Copy link
Contributor

@M0roSan M0roSan commented Mar 19, 2022

as described in the issue, only adding token_type in BaseTokenStorage and it'll do population.

TODO: switch to create group/project access token based on token_type field

haven't tested with gitlab instance nor vault instance yet

close #36

@codecov-commenter
Copy link

codecov-commenter commented Mar 19, 2022 •
edited
Loading

Codecov Report

Merging #50 (050942e) into main (c9ede88) will decrease coverage by 3.53%.
The diff coverage is 22.85%.

@@            Coverage Diff             @@
##             main      #50      +/-   ##
==========================================
- Coverage   59.45%   55.92%   -3.54%     
==========================================
  Files          10       10              
  Lines         550      608      +58     
==========================================
+ Hits          327      340      +13     
- Misses        195      240      +45     
  Partials       28       28
Flag Coverage Δ
unittests 55.92% <22.85%> (-3.54%) ⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

Impacted Files Coverage Δ
plugin/gitlab_client.go 25.45% <0.00%> (-11.39%) ⬇️
plugin/path_config.go 82.14% <ø> (ø)
plugin/path_token.go 30.13% <5.26%> (-6.53%) ⬇️
plugin/path_token_role.go 55.00% <33.33%> (ø)
plugin/token.go 48.71% <44.44%> (-2.27%) ⬇️
plugin/path_role.go 75.37% <50.00%> (+0.18%) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update c9ede88...050942e. Read the comment docs.

EvertonSA
EvertonSA approved these changes Mar 1, 2023
View reviewed changes
Copy link

@EvertonSA EvertonSA left a comment

Choose a reason for hiding this comment

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

there is some linting issues but it looks very cool, i really wanna try this feature

@M0roSan
Copy link
Contributor Author

M0roSan commented Apr 17, 2023

@nickshine Hi. is this still used and maintained outside here? and also, is it compatible with the running gitlab versions (I don't know which version but based on version compatibility, this should work). I'm more than happy to hand this someone who's actively using and potentially maintaining it

@nickshine
Copy link
Member

nickshine commented Apr 18, 2023 •
edited
Loading

Hey @M0roSan , yep this is still used and compatible with GitLab 15.x. Group access token feature hasn't been implemented b/c nobody has been asking for it, at least not that I've seen.

@nickshine
Copy link
Member

I'll take a look at this and see if we can test it out internally (and also update CI).

torfjor
torfjor reviewed Jun 8, 2023
View reviewed changes
Copy link

@torfjor torfjor left a comment

Choose a reason for hiding this comment

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

PGS is looking to use this. Initial testing shows that it works with our self-hosted GitLab instance (15.11.2-ee)

plugin/token.go
func (baseTokenStorage *BaseTokenStorageEntry) createAccessToken(gc Client, expiresAt time.Time) (data map[string]interface{}, err error) {
switch baseTokenStorage.TokenType {
case tokenTypeGroup:
gat, err := gc.CreateGroupAccessToken(baseTokenStorage, &expiresAt)
Copy link

Choose a reason for hiding this comment

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

This shadows err

plugin/token.go
}
data = groupTokenDetails(gat)
case tokenTypeProject:
pat, err := gc.CreateProjectAccessToken(baseTokenStorage, &expiresAt)
Copy link

Choose a reason for hiding this comment

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

Also shadows err

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

@torfjor torfjor torfjor left review comments

@EvertonSA EvertonSA EvertonSA approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

support for group level access token
5 participants
@M0roSan @codecov-commenter @nickshine @torfjor @EvertonSA