Merge pull request #31810 from backstage/blam/dcr-state

`DCR`: Change internal `state` column to `text`
This commit is contained in:
Fredrik Adelöw
2025-11-18 13:16:50 +01:00
committed by GitHub
4 changed files with 128 additions and 1 deletions
@@ -0,0 +1,41 @@
/*
* Copyright 2025 The Backstage Authors
*
* Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License.
* You may obtain a copy of the License at
*
* http://www.apache.org/licenses/LICENSE-2.0
*
* Unless required by applicable law or agreed to in writing, software
* distributed under the License is distributed on an "AS IS" BASIS,
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
* See the License for the specific language governing permissions and
* limitations under the License.
*/
// @ts-check
/**
* @param {import('knex').Knex} knex
*/
exports.up = async function up(knex) {
await knex.schema.alterTable('oauth_authorization_sessions', table => {
table.text('state', 'longtext').nullable().alter();
});
};
/**
* @param {import('knex').Knex} knex
*/
exports.down = async function down(knex) {
// Delete sessions with state > 255 chars since they won't fit in varchar(255)
// These sessions would be unusable with truncated state anyway
await knex('oauth_authorization_sessions')
.whereRaw('LENGTH(state) > 255')
.delete();
await knex.schema.alterTable('oauth_authorization_sessions', table => {
table.string('state').nullable().alter();
});
};
+1 -1
View File
@@ -18,7 +18,7 @@
| `redirect_uri` | `text` | false | - | - |
| `response_type` | `character varying` | false | 255 | - |
| `scope` | `text` | true | - | - |
| `state` | `character varying` | true | 255 | - |
| `state` | `text` | true | - | - |
| `status` | `text` | true | - | `'pending'::text` |
| `user_entity_ref` | `character varying` | true | 255 | - |
@@ -304,4 +304,85 @@ describe('migrations', () => {
await knex.destroy();
},
);
it.each(databases.eachSupportedId())(
'20251118120000_oauth_state_text.js, %p',
async databaseId => {
const knex = await databases.init(databaseId);
await migrateUntilBefore(knex, '20251118120000_oauth_state_text.js');
// First create a client for the foreign key constraint
await knex
.insert({
client_id: 'test-client-id',
client_secret: 'test-client-secret',
client_name: 'Test Client',
response_types: JSON.stringify(['code']),
grant_types: JSON.stringify(['authorization_code']),
redirect_uris: JSON.stringify(['https://example.com/callback']),
})
.into('oidc_clients');
// Insert a session with state before migration
const existingState = 'existing-short-state';
await knex
.insert({
id: 'test-existing-session',
client_id: 'test-client-id',
redirect_uri: 'https://example.com/callback',
state: existingState,
response_type: 'code',
status: 'pending',
expires_at: new Date(Date.now() + 3600000),
})
.into('oauth_authorization_sessions');
// Apply the migration that changes state to TEXT
await migrateUpOnce(knex);
// Verify existing state persists after migration
await expect(
knex('oauth_authorization_sessions')
.where('id', 'test-existing-session')
.first(),
).resolves.toEqual(
expect.objectContaining({
id: 'test-existing-session',
state: existingState,
}),
);
// Test inserting a state parameter longer than 255 characters
// This is based on the real-world example from the issue
const longState = 'a'.repeat(280);
await knex
.insert({
id: 'test-long-state-session',
client_id: 'test-client-id',
redirect_uri: 'https://example.com/callback',
state: longState,
response_type: 'code',
status: 'pending',
expires_at: new Date(Date.now() + 3600000),
})
.into('oauth_authorization_sessions');
await expect(
knex('oauth_authorization_sessions')
.where('id', 'test-long-state-session')
.first(),
).resolves.toEqual(
expect.objectContaining({
id: 'test-long-state-session',
state: longState,
}),
);
await migrateDownOnce(knex);
await knex.destroy();
},
);
});