Merge branch 'main' into add-priority-to-ProtectedBranch

Author: 6543

routers/api/v1/api.go

@@ -321,7 +321,7 @@ func tokenRequiresScopes(requiredScopeCategories ...auth_model.AccessTokenScopeC
 		}
 
 		if !allow {
-			ctx.Error(http.StatusForbidden, "tokenRequiresScope", fmt.Sprintf("token does not have at least one of required scope(s): %v", requiredScopes))
+			ctx.Error(http.StatusForbidden, "tokenRequiresScope", fmt.Sprintf("token does not have at least one of required scope(s), required=%v, token scope=%v", requiredScopes, scope))
 			return
 		}
 

services/oauth2_provider/access_token.go

@@ -74,26 +74,32 @@ type AccessTokenResponse struct {
 // GrantAdditionalScopes returns valid scopes coming from grant
 func GrantAdditionalScopes(grantScopes string) auth.AccessTokenScope {
 	// scopes_supported from templates/user/auth/oidc_wellknown.tmpl
-	scopesSupported := []string{
+	generalScopesSupported := []string{
 		"openid",
 		"profile",
 		"email",
 		"groups",
 	}
 
-	var tokenScopes []string
-	for _, tokenScope := range strings.Split(grantScopes, " ") {
-		if slices.Index(scopesSupported, tokenScope) == -1 {
-			tokenScopes = append(tokenScopes, tokenScope)
+	var accessScopes []string // the scopes for access control, but not for general information
+	for _, scope := range strings.Split(grantScopes, " ") {
+		if scope != "" && !slices.Contains(generalScopesSupported, scope) {
+			accessScopes = append(accessScopes, scope)
 		}
 	}
 
 	// since version 1.22, access tokens grant full access to the API
 	// with this access is reduced only if additional scopes are provided
-	accessTokenScope := auth.AccessTokenScope(strings.Join(tokenScopes, ","))
-	if accessTokenWithAdditionalScopes, err := accessTokenScope.Normalize(); err == nil && len(tokenScopes) > 0 {
-		return accessTokenWithAdditionalScopes
+	if len(accessScopes) > 0 {
+		accessTokenScope := auth.AccessTokenScope(strings.Join(accessScopes, ","))
+		if normalizedAccessTokenScope, err := accessTokenScope.Normalize(); err == nil {
+			return normalizedAccessTokenScope
+		}
+		// TODO: if there are invalid access scopes (err != nil),
+		// then it is treated as "all", maybe in the future we should make it stricter to return an error
+		// at the moment, to avoid breaking 1.22 behavior, invalid tokens are also treated as "all"
 	}
+	// fallback, empty access scope is treated as "all" access
 	return auth.AccessTokenScopeAll
 }
 

services/oauth2_provider/additional_scopes_test.go

@@ -14,6 +14,7 @@ func TestGrantAdditionalScopes(t *testing.T) {
 		grantScopes    string
 		expectedScopes string
 	}{
+		{"", "all"}, // for old tokens without scope, treat it as "all"
 		{"openid profile email", "all"},
 		{"openid profile email groups", "all"},
 		{"openid profile email all", "all"},
@@ -22,12 +23,14 @@ func TestGrantAdditionalScopes(t *testing.T) {
 		{"read:user read:repository", "read:repository,read:user"},
 		{"read:user write:issue public-only", "public-only,write:issue,read:user"},
 		{"openid profile email read:user", "read:user"},
+
+		// TODO: at the moment invalid tokens are treated as "all" to avoid breaking 1.22 behavior (more details are in GrantAdditionalScopes)
 		{"read:invalid_scope", "all"},
 		{"read:invalid_scope,write:scope_invalid,just-plain-wrong", "all"},
 	}
 
 	for _, test := range tests {
-		t.Run(test.grantScopes, func(t *testing.T) {
+		t.Run("scope:"+test.grantScopes, func(t *testing.T) {
 			result := GrantAdditionalScopes(test.grantScopes)
 			assert.Equal(t, test.expectedScopes, string(result))
 		})

templates/repo/diff/box.tmpl

@@ -164,24 +164,22 @@
 										<input type="checkbox" name="{{$file.GetDiffFileName}}" autocomplete="off"{{if $file.IsViewed}} checked{{end}}> {{ctx.Locale.Tr "repo.pulls.has_viewed_file"}}
 									</label>
 								{{end}}
-								<div class="ui dropdown basic">
-									{{svg "octicon-kebab-horizontal" 18 "icon tw-mx-2"}}
-									<div class="ui menu">
-										{{if not (or $file.IsIncomplete $file.IsBin $file.IsSubmodule)}}
-											<button class="unescape-button item">{{ctx.Locale.Tr "repo.unescape_control_characters"}}</button>
-											<button class="escape-button tw-hidden item">{{ctx.Locale.Tr "repo.escape_control_characters"}}</button>
-										{{end}}
-										{{if and (not $file.IsSubmodule) (not $.PageIsWiki)}}
-											{{if $file.IsDeleted}}
-												<a class="item" rel="nofollow" href="{{$.BeforeSourcePath}}/{{PathEscapeSegments .Name}}">{{ctx.Locale.Tr "repo.diff.view_file"}}</a>
-											{{else}}
-												<a class="item" rel="nofollow" href="{{$.SourcePath}}/{{PathEscapeSegments .Name}}">{{ctx.Locale.Tr "repo.diff.view_file"}}</a>
-												{{if and $.Repository.CanEnableEditor $.CanEditFile (not $file.IsLFSFile) (not $file.IsBin)}}
-													<a class="item" rel="nofollow" href="{{$.HeadRepoLink}}/_edit/{{PathEscapeSegments $.HeadBranchName}}/{{PathEscapeSegments $file.Name}}?return_uri={{print $.BackToLink "#diff-" $file.NameHash | QueryEscape}}">{{ctx.Locale.Tr "repo.editor.edit_this_file"}}</a>
-												{{end}}
+								<button class="btn diff-header-popup-btn tw-p-1">{{svg "octicon-kebab-horizontal" 18}}</button>
+								<div class="tippy-target">
+									{{if not (or $file.IsIncomplete $file.IsBin $file.IsSubmodule)}}
+										<button class="unescape-button item" data-file-content-elem-id="diff-{{$file.NameHash}}">{{ctx.Locale.Tr "repo.unescape_control_characters"}}</button>
+										<button class="escape-button tw-hidden item" data-file-content-elem-id="diff-{{$file.NameHash}}">{{ctx.Locale.Tr "repo.escape_control_characters"}}</button>
+									{{end}}
+									{{if and (not $file.IsSubmodule) (not $.PageIsWiki)}}
+										{{if $file.IsDeleted}}
+											<a class="item" rel="nofollow" href="{{$.BeforeSourcePath}}/{{PathEscapeSegments .Name}}">{{ctx.Locale.Tr "repo.diff.view_file"}}</a>
+										{{else}}
+											<a class="item" rel="nofollow" href="{{$.SourcePath}}/{{PathEscapeSegments .Name}}">{{ctx.Locale.Tr "repo.diff.view_file"}}</a>
+											{{if and $.Repository.CanEnableEditor $.CanEditFile (not $file.IsLFSFile) (not $file.IsBin)}}
+												<a class="item" rel="nofollow" href="{{$.HeadRepoLink}}/_edit/{{PathEscapeSegments $.HeadBranchName}}/{{PathEscapeSegments $file.Name}}?return_uri={{print $.BackToLink "#diff-" $file.NameHash | QueryEscape}}">{{ctx.Locale.Tr "repo.editor.edit_this_file"}}</a>
 											{{end}}
 										{{end}}
-									</div>
+									{{end}}
 								</div>
 							</div>
 						</h4>

tests/integration/oauth_test.go

@@ -565,7 +565,7 @@ func TestOAuth_GrantScopesReadUserFailRepos(t *testing.T) {
 
 	errorParsed := new(errorResponse)
 	require.NoError(t, json.Unmarshal(errorResp.Body.Bytes(), errorParsed))
-	assert.Contains(t, errorParsed.Message, "token does not have at least one of required scope(s): [read:repository]")
+	assert.Contains(t, errorParsed.Message, "token does not have at least one of required scope(s), required=[read:repository]")
 }
 
 func TestOAuth_GrantScopesReadRepositoryFailOrganization(t *testing.T) {
@@ -708,7 +708,7 @@ func TestOAuth_GrantScopesReadRepositoryFailOrganization(t *testing.T) {
 
 	errorParsed := new(errorResponse)
 	require.NoError(t, json.Unmarshal(errorResp.Body.Bytes(), errorParsed))
-	assert.Contains(t, errorParsed.Message, "token does not have at least one of required scope(s): [read:user read:organization]")
+	assert.Contains(t, errorParsed.Message, "token does not have at least one of required scope(s), required=[read:user read:organization]")
 }
 
 func TestOAuth_GrantScopesClaimPublicOnlyGroups(t *testing.T) {

tests/integration/pull_compare_test.go

@@ -97,7 +97,7 @@ func TestPullCompare_EnableAllowEditsFromMaintainer(t *testing.T) {
 		user2Session := loginUser(t, "user2")
 		resp = user2Session.MakeRequest(t, NewRequest(t, "GET", fmt.Sprintf("%s/files", prURL)), http.StatusOK)
 		htmlDoc := NewHTMLParser(t, resp.Body)
-		nodes := htmlDoc.doc.Find(".diff-file-box[data-new-filename=\"README.md\"] .diff-file-header-actions .dropdown .menu a")
+		nodes := htmlDoc.doc.Find(".diff-file-box[data-new-filename=\"README.md\"] .diff-file-header-actions .tippy-target a")
 		if assert.Equal(t, 1, nodes.Length()) {
 			// there is only "View File" button, no "Edit File" button
 			assert.Equal(t, "View File", nodes.First().Text())
@@ -121,7 +121,7 @@ func TestPullCompare_EnableAllowEditsFromMaintainer(t *testing.T) {
 		// user2 (admin of repo3) goes to the PR files page again
 		resp = user2Session.MakeRequest(t, NewRequest(t, "GET", fmt.Sprintf("%s/files", prURL)), http.StatusOK)
 		htmlDoc = NewHTMLParser(t, resp.Body)
-		nodes = htmlDoc.doc.Find(".diff-file-box[data-new-filename=\"README.md\"] .diff-file-header-actions .dropdown .menu a")
+		nodes = htmlDoc.doc.Find(".diff-file-box[data-new-filename=\"README.md\"] .diff-file-header-actions .tippy-target a")
 		if assert.Equal(t, 2, nodes.Length()) {
 			// there are "View File" button and "Edit File" button
 			assert.Equal(t, "View File", nodes.First().Text())

web_src/css/modules/tippy.css

@@ -77,8 +77,10 @@
   align-items: center;
   padding: 9px 18px;
   color: inherit;
+  background: inherit;
   text-decoration: none;
   gap: 10px;
+  width: 100%;
 }
 
 .tippy-box[data-theme="menu"] .item:hover {

web_src/js/features/repo-diff.ts

@@ -18,6 +18,7 @@ import {
 } from '../utils/dom.ts';
 import {POST, GET} from '../modules/fetch.ts';
 import {fomanticQuery} from '../modules/fomantic/base.ts';
+import {createTippy} from '../modules/tippy.ts';
 
 const {pageData, i18n} = window.config;
 
@@ -140,12 +141,22 @@ export function initRepoDiffConversationNav() {
   });
 }
 
+function initDiffHeaderPopup() {
+  for (const btn of document.querySelectorAll('.diff-header-popup-btn:not([data-header-popup-initialized])')) {
+    btn.setAttribute('data-header-popup-initialized', '');
+    const popup = btn.nextElementSibling;
+    if (!popup?.matches('.tippy-target')) throw new Error('Popup element not found');
+    createTippy(btn, {content: popup, theme: 'menu', placement: 'bottom', trigger: 'click', interactive: true, hideOnClick: true});
+  }
+}
+
 // Will be called when the show more (files) button has been pressed
 function onShowMoreFiles() {
   initRepoIssueContentHistory();
   initViewedCheckboxListenerFor();
   countAndUpdateViewedFiles();
   initImageDiff();
+  initDiffHeaderPopup();
 }
 
 export async function loadMoreFiles(url) {
@@ -221,6 +232,7 @@ export function initRepoDiffView() {
   initDiffFileList();
   initDiffCommitSelect();
   initRepoDiffShowMore();
+  initDiffHeaderPopup();
   initRepoDiffFileViewToggle();
   initViewedCheckboxListenerFor();
   initExpandAndCollapseFilesButton();

web_src/js/features/repo-unicode-escape.ts

@@ -1,13 +1,13 @@
-import {hideElem, queryElemSiblings, showElem, toggleElem} from '../utils/dom.ts';
+import {addDelegatedEventListener, hideElem, queryElemSiblings, showElem, toggleElem} from '../utils/dom.ts';
 
 export function initUnicodeEscapeButton() {
-  document.addEventListener('click', (e) => {
-    const btn = e.target.closest('.escape-button, .unescape-button, .toggle-escape-button');
-    if (!btn) return;
-
+  addDelegatedEventListener(document, 'click', '.escape-button, .unescape-button, .toggle-escape-button', (btn, e) => {
     e.preventDefault();
 
-    const fileContent = btn.closest('.file-content, .non-diff-file-content');
+    const fileContentElemId = btn.getAttribute('data-file-content-elem-id');
+    const fileContent = fileContentElemId ?
+      document.querySelector(`#${fileContentElemId}`) :
+      btn.closest('.file-content, .non-diff-file-content');
     const fileView = fileContent?.querySelectorAll('.file-code, .file-view');
     if (btn.matches('.escape-button')) {
       for (const el of fileView) el.classList.add('unicode-escaped');

web_src/js/markup/mermaid.ts

@@ -58,16 +58,12 @@ export async function renderMermaid(): Promise<void> {
       mermaidBlock.append(btn);
 
       const updateIframeHeight = () => {
-        iframe.style.height = `${iframe.contentWindow.document.body.clientHeight}px`;
+        const body = iframe.contentWindow?.document?.body;
+        if (body) {
+          iframe.style.height = `${body.clientHeight}px`;
+        }
       };
 
-      // update height when element's visibility state changes, for example when the diagram is inside
-      // a <details> + <summary> block and the <details> block becomes visible upon user interaction, it
-      // would initially set a incorrect height and the correct height is set during this callback.
-      (new IntersectionObserver(() => {
-        updateIframeHeight();
-      }, {root: document.documentElement})).observe(iframe);
-
       iframe.addEventListener('load', () => {
         pre.replaceWith(mermaidBlock);
         mermaidBlock.classList.remove('tw-hidden');
@@ -76,6 +72,13 @@ export async function renderMermaid(): Promise<void> {
           mermaidBlock.classList.remove('is-loading');
           iframe.classList.remove('tw-invisible');
         }, 0);
+
+        // update height when element's visibility state changes, for example when the diagram is inside
+        // a <details> + <summary> block and the <details> block becomes visible upon user interaction, it
+        // would initially set a incorrect height and the correct height is set during this callback.
+        (new IntersectionObserver(() => {
+          updateIframeHeight();
+        }, {root: document.documentElement})).observe(iframe);
       });
 
       document.body.append(mermaidBlock);