mirror of
https://codeberg.org/forgejo/forgejo.git
synced 2026-05-13 06:20:24 +00:00
feat(ui): display repositories accessible by repo-specific access tokens (#11604)
When an access token is repository specific, display the repositories that it can access when expanded in the UI (token **test** in this screenshot):  Default, collapsed view is unchanged:  Bulk loading of repositories is refactored out of the access token API endpoint into a `BulkGetRepositoriesForAccessTokens` method that can be used in both this UI, and the original API location. ## Checklist The [contributor guide](https://forgejo.org/docs/next/contributor/) contains information that will be helpful to first time contributors. There also are a few [conditions for merging Pull Requests in Forgejo repositories](https://codeberg.org/forgejo/governance/src/branch/main/PullRequestsAgreement.md). You are also welcome to join the [Forgejo development chatroom](https://matrix.to/#/#forgejo-development:matrix.org). ### Tests for Go changes (can be removed for JavaScript changes) - I added test coverage for Go changes... - [ ] in their respective `*_test.go` for unit tests. - [x] in the `tests/integration` directory if it involves interactions with a live Forgejo server. - I ran... - [x] `make pr-go` before pushing ### Documentation - [ ] I created a pull request [to the documentation](https://codeberg.org/forgejo/docs) to explain to Forgejo users how to use this change. - [x] I did not document these changes and I do not expect someone else to do it. ### Release notes - [x] This change will be noticed by a Forgejo user or admin (feature, bug fix, performance, etc.). I suggest to include a release note for this change. - [ ] This change is not visible to a Forgejo user or admin (refactor, dependency upgrade, etc.). I think there is no need to add a release note for this change. Reviewed-on: https://codeberg.org/forgejo/forgejo/pulls/11604 Reviewed-by: Andreas Ahlenstorf <aahlenst@noreply.codeberg.org> Co-authored-by: Mathieu Fenniak <mathieu@fenniak.net> Co-committed-by: Mathieu Fenniak <mathieu@fenniak.net>
This commit is contained in:
parent
8b57cfc152
commit
6e804c8b1b
6 changed files with 158 additions and 43 deletions
|
|
@ -15,10 +15,12 @@ import (
|
|||
"strconv"
|
||||
"strings"
|
||||
|
||||
auth_model "forgejo.org/models/auth"
|
||||
"forgejo.org/models/db"
|
||||
"forgejo.org/models/unit"
|
||||
user_model "forgejo.org/models/user"
|
||||
"forgejo.org/modules/cache"
|
||||
"forgejo.org/modules/container"
|
||||
"forgejo.org/modules/git"
|
||||
"forgejo.org/modules/log"
|
||||
"forgejo.org/modules/markup"
|
||||
|
|
@ -1001,3 +1003,55 @@ func UpdateRepoIssueNumbers(ctx context.Context, repoID int64, isPull, isClosed
|
|||
})
|
||||
return nil
|
||||
}
|
||||
|
||||
// Bulk load of all the repo_model.Repository objects for the repository resources that can be accessed by the given
|
||||
// access tokens. Any access tokens which are not repository-specific tokens will not be present in the map. An
|
||||
// optional filter function can be used to remove repositories (based upon a user visibility check, for example) before
|
||||
// the map is constructed -- return `true` for repos to include.
|
||||
func BulkGetRepositoriesForAccessTokens(ctx context.Context, tokens []*auth_model.AccessToken, filter func(*Repository) (bool, error)) (map[int64][]*Repository, error) {
|
||||
// Load all the AccessTokenResourceRepo for the tokens that we're returning:
|
||||
allRepoIDs := container.Set[int64]{}
|
||||
repoResourcesByTokenID, err := auth_model.GetRepositoriesAccessibleWithTokens(ctx, tokens)
|
||||
if err != nil {
|
||||
return nil, fmt.Errorf("failed to fetch repositories for tokens: %w", err)
|
||||
}
|
||||
|
||||
// Load all the Repository models that are referenced by the AccessTokenResourceRepo's:
|
||||
for _, repoResources := range repoResourcesByTokenID {
|
||||
for _, repoResource := range repoResources {
|
||||
allRepoIDs.Add(repoResource.RepoID)
|
||||
}
|
||||
}
|
||||
reposByID, err := GetRepositoriesMapByIDs(ctx, allRepoIDs.Slice())
|
||||
if err != nil {
|
||||
return nil, fmt.Errorf("failed to fetch repositories: %w", err)
|
||||
}
|
||||
|
||||
if filter != nil {
|
||||
// Rebuild reposByID, filtering it with the provided filter function. It's more efficient to do this here,
|
||||
// rather than returning the data and allowing the caller to filter it, because this guarantees one invocation
|
||||
// per repository. `reposByTokenID` could have the same repository referenced by multiple access tokens.
|
||||
tmp := reposByID
|
||||
reposByID = make(map[int64]*Repository, len(tmp))
|
||||
for id, repo := range tmp {
|
||||
if ok, err := filter(repo); err != nil {
|
||||
return nil, fmt.Errorf("error filtering repo %d: %w", repo.ID, err)
|
||||
} else if ok {
|
||||
reposByID[id] = repo
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
// Prepare a lookup map to access the repositories by token ID:
|
||||
reposByTokenID := make(map[int64][]*Repository)
|
||||
for tokenID, repoResources := range repoResourcesByTokenID {
|
||||
for _, repoResource := range repoResources {
|
||||
repo, ok := reposByID[repoResource.RepoID]
|
||||
if ok {
|
||||
reposByTokenID[tokenID] = append(reposByTokenID[tokenID], repo)
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
return reposByTokenID, nil
|
||||
}
|
||||
|
|
|
|||
|
|
@ -191,6 +191,7 @@
|
|||
"settings.twofa_reenroll": "Re-enroll two-factor authentication",
|
||||
"settings.twofa_reenroll.description": "Re-enroll your two-factor authentication",
|
||||
"settings.must_enable_2fa": "This Forgejo instance requires users to enable two-factor authentication before they can access their accounts.",
|
||||
"settings.specific_repo_access": "Repository Access",
|
||||
"error.must_enable_2fa": "This Forgejo instance requires users to enable two-factor authentication before they can access their accounts. Enable it at: %s",
|
||||
"avatar.constraints_hint": "Custom avatar may not exceed %[1]s in size or be larger than %[2]dx%[3]d pixels",
|
||||
"user.ghost.tooltip": "This user has been deleted, or cannot be matched.",
|
||||
|
|
|
|||
|
|
@ -18,7 +18,6 @@ import (
|
|||
"forgejo.org/models/db"
|
||||
access_model "forgejo.org/models/perm/access"
|
||||
repo_model "forgejo.org/models/repo"
|
||||
"forgejo.org/modules/container"
|
||||
"forgejo.org/modules/optional"
|
||||
api "forgejo.org/modules/structs"
|
||||
"forgejo.org/modules/web"
|
||||
|
|
@ -66,42 +65,34 @@ func ListAccessTokens(ctx *context.APIContext) {
|
|||
}
|
||||
|
||||
// Load all the AccessTokenResourceRepo for the tokens that we're returning:
|
||||
allRepoIDs := container.Set[int64]{}
|
||||
repoResourcesByTokenID, err := auth_model.GetRepositoriesAccessibleWithTokens(ctx, tokens)
|
||||
if err != nil {
|
||||
ctx.InternalServerError(err)
|
||||
return
|
||||
}
|
||||
|
||||
// Load all the Repository models that are referenced by the AccessTokenResourceRepo's:
|
||||
for _, repoResources := range repoResourcesByTokenID {
|
||||
for _, repoResource := range repoResources {
|
||||
allRepoIDs.Add(repoResource.RepoID)
|
||||
}
|
||||
}
|
||||
reposByID, err := repo_model.GetRepositoriesMapByIDs(ctx, allRepoIDs.Slice())
|
||||
if err != nil {
|
||||
ctx.InternalServerError(err)
|
||||
return
|
||||
}
|
||||
|
||||
// Prepare a lookup map to access the repositories by token ID:
|
||||
reposByTokenID := make(map[int64][]*api.RepositoryMeta)
|
||||
for tokenID, repoResources := range repoResourcesByTokenID {
|
||||
for _, repoResource := range repoResources {
|
||||
repo, ok := reposByID[repoResource.RepoID]
|
||||
if !ok {
|
||||
// Shouldn't be possible with the foreign key on `AccessTokenResourceRepo` to the repository table.
|
||||
ctx.Error(http.StatusInternalServerError, "reposById", "missing repository")
|
||||
return
|
||||
repoModelsByTokenID, err := repo_model.BulkGetRepositoriesForAccessTokens(ctx, tokens,
|
||||
func(repo *repo_model.Repository) (bool, error) {
|
||||
// Repos associated with a repo-specific access token should already be visible to the token owner, but it's
|
||||
// possible that access has changed, such as a removed collaborator on a repo -- don't provide info on that
|
||||
// repo if so.
|
||||
permission, err := access_model.GetUserRepoPermissionWithReducer(ctx, repo, ctx.Doer, ctx.Reducer)
|
||||
if err != nil {
|
||||
return false, err
|
||||
}
|
||||
reposByTokenID[tokenID] = append(reposByTokenID[tokenID], &api.RepositoryMeta{
|
||||
return permission.HasAccess(), nil
|
||||
})
|
||||
if err != nil {
|
||||
ctx.InternalServerError(err)
|
||||
return
|
||||
}
|
||||
// Convert map[int64]*Repository -> map[int64]*RepositoryMeta...
|
||||
reposByTokenID := make(map[int64][]*api.RepositoryMeta)
|
||||
for tokenID, repoModels := range repoModelsByTokenID {
|
||||
repos := make([]*api.RepositoryMeta, len(repoModels))
|
||||
for i, repo := range repoModels {
|
||||
repos[i] = &api.RepositoryMeta{
|
||||
ID: repo.ID,
|
||||
Name: repo.Name,
|
||||
Owner: repo.OwnerName,
|
||||
FullName: repo.FullName(),
|
||||
})
|
||||
}
|
||||
}
|
||||
reposByTokenID[tokenID] = repos
|
||||
}
|
||||
|
||||
apiTokens := make([]*api.AccessToken, len(tokens))
|
||||
|
|
|
|||
|
|
@ -9,6 +9,8 @@ import (
|
|||
|
||||
auth_model "forgejo.org/models/auth"
|
||||
"forgejo.org/models/db"
|
||||
access_model "forgejo.org/models/perm/access"
|
||||
repo_model "forgejo.org/models/repo"
|
||||
"forgejo.org/modules/base"
|
||||
"forgejo.org/modules/log"
|
||||
"forgejo.org/modules/setting"
|
||||
|
|
@ -112,6 +114,11 @@ func RegenerateApplication(ctx *context.Context) {
|
|||
ctx.JSONRedirect(setting.AppSubURL + "/user/settings/applications")
|
||||
}
|
||||
|
||||
type TokenWithResources struct {
|
||||
Token *auth_model.AccessToken
|
||||
Repositories []*repo_model.Repository
|
||||
}
|
||||
|
||||
func loadApplicationsData(ctx *context.Context) {
|
||||
ctx.Data["AccessTokenScopePublicOnly"] = auth_model.AccessTokenScopePublicOnly
|
||||
tokens, err := db.Find[auth_model.AccessToken](ctx, auth_model.ListAccessTokensOptions{UserID: ctx.Doer.ID})
|
||||
|
|
@ -119,7 +126,33 @@ func loadApplicationsData(ctx *context.Context) {
|
|||
ctx.ServerError("ListAccessTokens", err)
|
||||
return
|
||||
}
|
||||
ctx.Data["Tokens"] = tokens
|
||||
|
||||
// Load all the AccessTokenResourceRepo for the tokens that we're returning:
|
||||
reposByTokenID, err := repo_model.BulkGetRepositoriesForAccessTokens(ctx, tokens,
|
||||
func(repo *repo_model.Repository) (bool, error) {
|
||||
// Repos associated with a repo-specific access token should already be visible to the token owner, but it's
|
||||
// possible that access has changed, such as a removed collaborator on a repo -- don't provide info on that
|
||||
// repo if so.
|
||||
permission, err := access_model.GetUserRepoPermission(ctx, repo, ctx.Doer)
|
||||
if err != nil {
|
||||
return false, err
|
||||
}
|
||||
return permission.HasAccess(), nil
|
||||
})
|
||||
if err != nil {
|
||||
ctx.ServerError("BulkGetRepositoriesForAccessTokens", err)
|
||||
return
|
||||
}
|
||||
|
||||
tokensWithResources := make([]*TokenWithResources, len(tokens))
|
||||
for i := range tokens {
|
||||
tokensWithResources[i] = &TokenWithResources{
|
||||
Token: tokens[i],
|
||||
Repositories: reposByTokenID[tokens[i].ID],
|
||||
}
|
||||
}
|
||||
|
||||
ctx.Data["TokensWithResources"] = tokensWithResources
|
||||
ctx.Data["EnableOAuth2"] = setting.OAuth2.Enabled
|
||||
ctx.Data["IsAdmin"] = ctx.Doer.IsAdmin
|
||||
if setting.OAuth2.Enabled {
|
||||
|
|
|
|||
|
|
@ -8,27 +8,34 @@
|
|||
<div class="flex-item">
|
||||
{{ctx.Locale.Tr "settings.tokens_desc"}}
|
||||
</div>
|
||||
{{range .Tokens}}
|
||||
{{range .TokensWithResources}}
|
||||
<div class="flex-item">
|
||||
<div class="flex-item-leading">
|
||||
<span class="text {{if .HasRecentActivity}}green{{end}}" {{if .HasRecentActivity}}data-tooltip-content="{{ctx.Locale.Tr "settings.token_state_desc"}}"{{end}}>
|
||||
<span class="text {{if .Token.HasRecentActivity}}green{{end}}" {{if .Token.HasRecentActivity}}data-tooltip-content="{{ctx.Locale.Tr "settings.token_state_desc"}}"{{end}}>
|
||||
{{svg "fontawesome-send" 32}}
|
||||
</span>
|
||||
</div>
|
||||
<div class="flex-item-main">
|
||||
<details>
|
||||
<summary><span class="flex-item-title">{{.Name}}</span></summary>
|
||||
<p class="tw-my-1">
|
||||
{{ctx.Locale.Tr "settings.repo_and_org_access"}}:
|
||||
{{if .DisplayPublicOnly}}
|
||||
<summary><span class="flex-item-title">{{.Token.Name}}</span></summary>
|
||||
{{if .Token.ResourceAllRepos}}
|
||||
<p class="tw-my-1">{{ctx.Locale.Tr "settings.repo_and_org_access"}}:</p>
|
||||
{{if .Token.DisplayPublicOnly}}
|
||||
{{ctx.Locale.Tr "settings.permissions_public_only"}}
|
||||
{{else}}
|
||||
{{ctx.Locale.Tr "settings.permissions_access_all"}}
|
||||
{{end}}
|
||||
</p>
|
||||
{{else}}
|
||||
<p class="tw-my-1">{{ctx.Locale.Tr "settings.specific_repo_access"}}:</p>
|
||||
<ul class="tw-my-1">
|
||||
{{range .Repositories}}
|
||||
<li><a href="{{.HTMLURL}}">{{.FullName}}</a></li>
|
||||
{{end}}
|
||||
</ul>
|
||||
{{end}}
|
||||
<p class="tw-my-1">{{ctx.Locale.Tr "settings.permissions_list"}}</p>
|
||||
<ul class="tw-my-1">
|
||||
{{range .Scope.StringSlice}}
|
||||
{{range .Token.Scope.StringSlice}}
|
||||
{{if (ne . $.AccessTokenScopePublicOnly)}}
|
||||
<li>{{.}}</li>
|
||||
{{end}}
|
||||
|
|
@ -36,15 +43,15 @@
|
|||
</ul>
|
||||
</details>
|
||||
<div class="flex-item-body">
|
||||
<p>{{ctx.Locale.Tr "settings.added_on" (DateUtils.AbsoluteShort .CreatedUnix)}} — {{svg "octicon-info"}} {{if .HasUsed}}{{ctx.Locale.Tr "settings.last_used"}} <span {{if .HasRecentActivity}}class="text green"{{end}}>{{DateUtils.AbsoluteShort .UpdatedUnix}}</span>{{else}}{{ctx.Locale.Tr "settings.no_activity"}}{{end}}</p>
|
||||
<p>{{ctx.Locale.Tr "settings.added_on" (DateUtils.AbsoluteShort .Token.CreatedUnix)}} — {{svg "octicon-info"}} {{if .Token.HasUsed}}{{ctx.Locale.Tr "settings.last_used"}} <span {{if .Token.HasRecentActivity}}class="text green"{{end}}>{{DateUtils.AbsoluteShort .Token.UpdatedUnix}}</span>{{else}}{{ctx.Locale.Tr "settings.no_activity"}}{{end}}</p>
|
||||
</div>
|
||||
</div>
|
||||
<div class="flex-item-trailing">
|
||||
<button class="ui primary tiny button delete-button" data-modal-id="regenerate-token" data-url="{{$.Link}}/regenerate" data-id="{{.ID}}">
|
||||
<button class="ui primary tiny button delete-button" data-modal-id="regenerate-token" data-url="{{$.Link}}/regenerate" data-id="{{.Token.ID}}">
|
||||
{{svg "octicon-issue-reopened" 16 "tw-mr-1"}}
|
||||
{{ctx.Locale.Tr "settings.regenerate_token"}}
|
||||
</button>
|
||||
<button class="ui red tiny button delete-button" data-modal-id="delete-token" data-url="{{$.Link}}/delete" data-id="{{.ID}}">
|
||||
<button class="ui red tiny button delete-button" data-modal-id="delete-token" data-url="{{$.Link}}/delete" data-id="{{.Token.ID}}">
|
||||
{{svg "octicon-trash" 16 "tw-mr-1"}}
|
||||
{{ctx.Locale.Tr "settings.delete_token"}}
|
||||
</button>
|
||||
|
|
|
|||
|
|
@ -281,6 +281,35 @@ func TestAccessTokenRegenerate(t *testing.T) {
|
|||
assert.NotEqual(t, "TestAccessToken", latestTokenName)
|
||||
}
|
||||
|
||||
func TestAccessTokenResourceRepos(t *testing.T) {
|
||||
defer tests.PrepareTestEnv(t)()
|
||||
|
||||
session := loginUser(t, "user2")
|
||||
|
||||
// Before creating a repo-specific access token, we shouldn't have the "Repository Access:" list in the personal
|
||||
// access token page:
|
||||
req := NewRequest(t, "GET", "/user/settings/applications")
|
||||
resp := session.MakeRequest(t, req, http.StatusOK)
|
||||
htmlDoc := NewHTMLParser(t, resp.Body)
|
||||
htmlDoc.AssertSelection(t, htmlDoc.FindByText(".user-setting-content p", "Repository Access:"), false)
|
||||
|
||||
// Then we create a repo-specific access token. We give it access to two repos, user2/repo2, but also user30/empty,
|
||||
// a private repo owned by someone else... We'll pretend user2 used to be a collaborator on this repo and
|
||||
// previously had access to view it, but doesn't anymore.
|
||||
createFineGrainedRepoAccessToken(t, "user2",
|
||||
[]auth_model.AccessTokenScope{auth_model.AccessTokenScopeReadUser},
|
||||
[]int64{2, 52},
|
||||
)
|
||||
|
||||
// Now we have "Repository Access:"...
|
||||
req = NewRequest(t, "GET", "/user/settings/applications")
|
||||
resp = session.MakeRequest(t, req, http.StatusOK)
|
||||
htmlDoc = NewHTMLParser(t, resp.Body)
|
||||
htmlDoc.AssertSelection(t, htmlDoc.FindByText(".user-setting-content p", "Repository Access:"), true)
|
||||
htmlDoc.AssertSelection(t, htmlDoc.FindByText(".user-setting-content a", "user2/repo2"), true) // link to repo
|
||||
htmlDoc.AssertSelection(t, htmlDoc.FindByText(".user-setting-content a", "user30/empty"), false) // missing - user2 has no visibility
|
||||
}
|
||||
|
||||
func findLatestTokenID(t *testing.T, session *TestSession) (string, int) {
|
||||
req := NewRequest(t, "GET", "/user/settings/applications")
|
||||
resp := session.MakeRequest(t, req, http.StatusOK)
|
||||
|
|
|
|||
Loading…
Add table
Add a link
Reference in a new issue