Skip to content

Commit 5739049

Browse files
committed
Merge remote-tracking branch 'origin/v16/dev'
2 parents 2b8146f + 4e74dbf commit 5739049

File tree

7 files changed

+54
-13
lines changed

7 files changed

+54
-13
lines changed

build/nightly-E2E-test-pipelines.yml

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -9,6 +9,7 @@ schedules:
99
branches:
1010
include:
1111
- v15/dev
12+
- v16/dev
1213
- main
1314
- v17/dev
1415

@@ -695,4 +696,4 @@ stages:
695696
--data "$PAYLOAD" \
696697
"$SLACK_WEBHOOK_URL"
697698
env:
698-
SLACK_WEBHOOK_URL: $(E2ESLACKWEBHOOKURL)
699+
SLACK_WEBHOOK_URL: $(E2ESLACKWEBHOOKURL)

src/Umbraco.Web.UI.Client/src/external/openid/src/redirect_based_handler.ts

Lines changed: 41 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -77,6 +77,41 @@ export class RedirectRequestHandler extends AuthorizationRequestHandler {
7777
});
7878
}
7979

80+
/**
81+
* Cleanup all stale authorization requests and configurations from storage.
82+
* This scans localStorage for any keys matching the appauth patterns and removes them,
83+
* including the authorization request handle key.
84+
*/
85+
public cleanupStaleAuthorizationData(): Promise<void> {
86+
// Check if we're in a browser environment with localStorage
87+
if (typeof window === 'undefined' || !window.localStorage) {
88+
return Promise.resolve();
89+
}
90+
91+
const keysToRemove: string[] = [];
92+
93+
// Scan localStorage for all appauth-related keys
94+
for (let i = 0; i < window.localStorage.length; i++) {
95+
const key = window.localStorage.key(i);
96+
if (
97+
key &&
98+
(key.includes('_appauth_authorization_request') ||
99+
key.includes('_appauth_authorization_service_configuration') ||
100+
key === AUTHORIZATION_REQUEST_HANDLE_KEY)
101+
) {
102+
keysToRemove.push(key);
103+
}
104+
}
105+
106+
// Remove all found stale keys
107+
const removePromises = keysToRemove.map((key) => this.storageBackend.removeItem(key));
108+
return Promise.all(removePromises).then(() => {
109+
if (keysToRemove.length > 0) {
110+
log(`Cleaned up ${keysToRemove.length} stale authorization data entries`);
111+
}
112+
});
113+
}
114+
80115
/**
81116
* Attempts to introspect the contents of storage backend and completes the
82117
* request.
@@ -119,12 +154,8 @@ export class RedirectRequestHandler extends AuthorizationRequestHandler {
119154
} else {
120155
authorizationResponse = new AuthorizationResponse({ code: code, state: state });
121156
}
122-
// cleanup state
123-
return Promise.all([
124-
this.storageBackend.removeItem(AUTHORIZATION_REQUEST_HANDLE_KEY),
125-
this.storageBackend.removeItem(authorizationRequestKey(handle)),
126-
this.storageBackend.removeItem(authorizationServiceConfigurationKey(handle)),
127-
]).then(() => {
157+
// cleanup all authorization data including current and stale entries
158+
return this.cleanupStaleAuthorizationData().then(() => {
128159
log('Delivering authorization response');
129160
return {
130161
request: request,
@@ -134,7 +165,10 @@ export class RedirectRequestHandler extends AuthorizationRequestHandler {
134165
});
135166
} else {
136167
log('Mismatched request (state and request_uri) dont match.');
137-
return Promise.resolve(null);
168+
// cleanup all authorization data even on mismatch to prevent stale PKCE data
169+
return this.cleanupStaleAuthorizationData().then(() => {
170+
return null;
171+
});
138172
}
139173
})
140174
);

src/Umbraco.Web.UI.Client/src/packages/core/auth/auth-flow.ts

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -247,6 +247,10 @@ export class UmbAuthFlow {
247247

248248
// clear the internal state
249249
this.#tokenResponse.setValue(undefined);
250+
251+
// Also cleanup any OAuth/PKCE artifacts that may still be in localStorage
252+
// This is a defense-in-depth measure during logout
253+
await this.#authorizationHandler.cleanupStaleAuthorizationData();
250254
}
251255

252256
/**

tests/Umbraco.Tests.AcceptanceTest/package.json

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -27,4 +27,4 @@
2727
"dotenv": "^16.3.1",
2828
"node-fetch": "^2.6.7"
2929
}
30-
}
30+
}

tests/Umbraco.Tests.AcceptanceTest/tests/DefaultConfig/Content/Clipboard/ClipboardBlockGridBlocks.spec.ts

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -106,7 +106,8 @@ test('can copy and paste a single block into the same document but different gro
106106
await umbracoUi.content.doesBlockEditorBlockWithNameContainValue(elementGroupName, elementPropertyName, ConstantHelper.inputTypes.tipTap, blockPropertyValue);
107107
});
108108

109-
test('can copy and paste a single block into another document', async ({umbracoApi, umbracoUi}) => {
109+
// Remove skip after this issue is resolved: https://github.com/umbraco/Umbraco-CMS/issues/20680
110+
test.skip('can copy and paste a single block into another document', async ({umbracoApi, umbracoUi}) => {
110111
// Arrange
111112
await umbracoApi.document.ensureNameNotExists(secondContentName);
112113
await umbracoApi.document.createDefaultDocumentWithABlockGridEditorAndBlockWithValue(contentName, documentTypeName, blockGridDataTypeName, elementTypeId, AliasHelper.toAlias(elementPropertyName), blockPropertyValue, richTextDataTypeUiAlias);

tests/Umbraco.Tests.AcceptanceTest/tests/DefaultConfig/Content/Clipboard/ClipboardBlockListBlocks.spec.ts

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -106,7 +106,8 @@ test('can copy and paste a single block into the same document but different gro
106106
await umbracoUi.content.doesBlockEditorBlockWithNameContainValue(elementGroupName, elementPropertyName, ConstantHelper.inputTypes.tipTap, blockPropertyValue);
107107
});
108108

109-
test('can copy and paste a single block into another document', async ({umbracoApi, umbracoUi}) => {
109+
// Remove skip after this issue is resolved: https://github.com/umbraco/Umbraco-CMS/issues/20680
110+
test.skip('can copy and paste a single block into another document', async ({umbracoApi, umbracoUi}) => {
110111
// Arrange
111112
await umbracoApi.document.ensureNameNotExists(secondContentName);
112113
await umbracoApi.document.createDefaultDocumentWithABlockListEditorAndBlockWithValue(contentName, documentTypeName, blockListDataTypeName, elementTypeId, AliasHelper.toAlias(elementPropertyName), blockPropertyValue, elementDataTypeUiAlias, groupName);

tests/Umbraco.Tests.AcceptanceTest/tests/DefaultConfig/Webhook/Webhook.spec.ts

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -14,7 +14,7 @@ test.afterEach(async ({umbracoApi}) => {
1414
await umbracoApi.webhook.ensureNameNotExists(webhookName);
1515
});
1616

17-
test('can create a webhook', {tag: '@release'}, async ({umbracoApi, umbracoUi}) => {
17+
test('can create a webhook', async ({umbracoApi, umbracoUi}) => {
1818
// Arrange
1919
const event = 'Content Deleted';
2020
const webhookSiteUrl = umbracoApi.webhook.webhookSiteUrl + webhookSiteToken;
@@ -122,7 +122,7 @@ test('can disable a webhook', async ({umbracoApi, umbracoUi}) => {
122122
await umbracoApi.webhook.isWebhookEnabled(webhookName, false);
123123
});
124124

125-
test('cannot remove all events from a webhook', {tag: '@release'}, async ({umbracoApi, umbracoUi}) => {
125+
test('cannot remove all events from a webhook', async ({umbracoApi, umbracoUi}) => {
126126
// Arrange
127127
const event = 'Content Deleted';
128128
await umbracoApi.webhook.createDefaultWebhook(webhookName, webhookSiteToken, event);

0 commit comments

Comments
 (0)