bitwarden / server

Bitwarden infrastructure/backend (API, database, Docker, etc).
https://bitwarden.com
Other
15.73k stars 1.32k forks source link

Unified self-hosted: OIDC login to accept invite fails with ExternalId > 50 chars #2891

Open obrienmd opened 1 year ago

obrienmd commented 1 year ago

Steps To Reproduce

As admin:

  1. Install unified self-hosted:beta Docker image (also tested with :dev)
  2. Configure OIDC SSO provider
  3. Configure SCIM (also seems to be the case with manually added users) and sync to create a user

As user:

  1. Click the e-mail link to accept invite
  2. Click to log in
  3. Enter e-mail address
  4. Click Enterprise SSO
  5. Successfully complete SSO flow

Expected Result

Invite is accepted and user onboarding flow continues as expected.

Actual Result

User sees error message, user state remains "Invited".

Error message in SSO logs:

info: Microsoft.EntityFrameworkCore.Infrastructure[10403]
      Entity Framework Core 6.0.12 initialized 'DatabaseContext' using provider 'Npgsql.EntityFrameworkCore.PostgreSQL:6.0.8+e68dfe8b5cbe4a26d20acc36def6187aa1cfdda3' with options: MigrationsAssembly=PostgresMigrations 
fail: Microsoft.EntityFrameworkCore.Database.Command[20102]
      Failed executing DbCommand (6ms) [Parameters=[@p0='?' (DbType = DateTime), @p1='?', @p2='?' (DbType = Guid), @p3='?' (DbType = Guid)], CommandType='Text', CommandTimeout='30']
      INSERT INTO "SsoUser" ("CreationDate", "ExternalId", "OrganizationId", "UserId")
      VALUES (@p0, @p1, @p2, @p3)
      RETURNING "Id";
fail: Microsoft.EntityFrameworkCore.Update[10000]
      An exception occurred in the database while saving changes for context type 'Bit.Infrastructure.EntityFramework.Repositories.DatabaseContext'.
      Microsoft.EntityFrameworkCore.DbUpdateException: An error occurred while saving the entity changes. See the inner exception for details.
       ---> Npgsql.PostgresException (0x80004005): 22001: value too long for type character varying(50)
         at Npgsql.Internal.NpgsqlConnector.<ReadMessage>g__ReadMessageLong|215_0(NpgsqlConnector connector, Boolean async, DataRowLoadingMode dataRowLoadingMode, Boolean readingNotifications, Boolean isReadingPrependedMessage)
         at Npgsql.NpgsqlDataReader.NextResult(Boolean async, Boolean isConsuming, CancellationToken cancellationToken)
         at Npgsql.NpgsqlCommand.ExecuteReader(CommandBehavior behavior, Boolean async, CancellationToken cancellationToken)
         at Npgsql.NpgsqlCommand.ExecuteReader(CommandBehavior behavior, Boolean async, CancellationToken cancellationToken)
         at Npgsql.NpgsqlCommand.ExecuteDbDataReaderAsync(CommandBehavior behavior, CancellationToken cancellationToken)
         at Microsoft.EntityFrameworkCore.Storage.RelationalCommand.ExecuteReaderAsync(RelationalCommandParameterObject parameterObject, CancellationToken cancellationToken)
         at Microsoft.EntityFrameworkCore.Storage.RelationalCommand.ExecuteReaderAsync(RelationalCommandParameterObject parameterObject, CancellationToken cancellationToken)
         at Microsoft.EntityFrameworkCore.Update.ReaderModificationCommandBatch.ExecuteAsync(IRelationalConnection connection, CancellationToken cancellationToken)
        Exception data:
          Severity: ERROR
          SqlState: 22001
          MessageText: value too long for type character varying(50)
          File: varchar.c
          Line: 632
          Routine: varchar
         --- End of inner exception stack trace ---
         at Microsoft.EntityFrameworkCore.Update.ReaderModificationCommandBatch.ExecuteAsync(IRelationalConnection connection, CancellationToken cancellationToken)
         at Microsoft.EntityFrameworkCore.Update.Internal.BatchExecutor.ExecuteAsync(IEnumerable`1 commandBatches, IRelationalConnection connection, CancellationToken cancellationToken)
         at Microsoft.EntityFrameworkCore.Update.Internal.BatchExecutor.ExecuteAsync(IEnumerable`1 commandBatches, IRelationalConnection connection, CancellationToken cancellationToken)
         at Microsoft.EntityFrameworkCore.Update.Internal.BatchExecutor.ExecuteAsync(IEnumerable`1 commandBatches, IRelationalConnection connection, CancellationToken cancellationToken)
         at Microsoft.EntityFrameworkCore.ChangeTracking.Internal.StateManager.SaveChangesAsync(IList`1 entriesToSave, CancellationToken cancellationToken)
         at Microsoft.EntityFrameworkCore.ChangeTracking.Internal.StateManager.SaveChangesAsync(StateManager stateManager, Boolean acceptAllChangesOnSuccess, CancellationToken cancellationToken)
         at Npgsql.EntityFrameworkCore.PostgreSQL.Storage.Internal.NpgsqlExecutionStrategy.ExecuteAsync[TState,TResult](TState state, Func`4 operation, Func`4 verifySucceeded, CancellationToken cancellationToken)
         at Microsoft.EntityFrameworkCore.DbContext.SaveChangesAsync(Boolean acceptAllChangesOnSuccess, CancellationToken cancellationToken)
      Microsoft.EntityFrameworkCore.DbUpdateException: An error occurred while saving the entity changes. See the inner exception for details.
       ---> Npgsql.PostgresException (0x80004005): 22001: value too long for type character varying(50)
         at Npgsql.Internal.NpgsqlConnector.<ReadMessage>g__ReadMessageLong|215_0(NpgsqlConnector connector, Boolean async, DataRowLoadingMode dataRowLoadingMode, Boolean readingNotifications, Boolean isReadingPrependedMessage)
         at Npgsql.NpgsqlDataReader.NextResult(Boolean async, Boolean isConsuming, CancellationToken cancellationToken)
         at Npgsql.NpgsqlCommand.ExecuteReader(CommandBehavior behavior, Boolean async, CancellationToken cancellationToken)
         at Npgsql.NpgsqlCommand.ExecuteReader(CommandBehavior behavior, Boolean async, CancellationToken cancellationToken)
         at Npgsql.NpgsqlCommand.ExecuteDbDataReaderAsync(CommandBehavior behavior, CancellationToken cancellationToken)
         at Microsoft.EntityFrameworkCore.Storage.RelationalCommand.ExecuteReaderAsync(RelationalCommandParameterObject parameterObject, CancellationToken cancellationToken)
         at Microsoft.EntityFrameworkCore.Storage.RelationalCommand.ExecuteReaderAsync(RelationalCommandParameterObject parameterObject, CancellationToken cancellationToken)
         at Microsoft.EntityFrameworkCore.Update.ReaderModificationCommandBatch.ExecuteAsync(IRelationalConnection connection, CancellationToken cancellationToken)
        Exception data:
          Severity: ERROR
          SqlState: 22001
          MessageText: value too long for type character varying(50)
          File: varchar.c
          Line: 632
          Routine: varchar
         --- End of inner exception stack trace ---
         at Microsoft.EntityFrameworkCore.Update.ReaderModificationCommandBatch.ExecuteAsync(IRelationalConnection connection, CancellationToken cancellationToken)
         at Microsoft.EntityFrameworkCore.Update.Internal.BatchExecutor.ExecuteAsync(IEnumerable`1 commandBatches, IRelationalConnection connection, CancellationToken cancellationToken)
         at Microsoft.EntityFrameworkCore.Update.Internal.BatchExecutor.ExecuteAsync(IEnumerable`1 commandBatches, IRelationalConnection connection, CancellationToken cancellationToken)
         at Microsoft.EntityFrameworkCore.Update.Internal.BatchExecutor.ExecuteAsync(IEnumerable`1 commandBatches, IRelationalConnection connection, CancellationToken cancellationToken)
         at Microsoft.EntityFrameworkCore.ChangeTracking.Internal.StateManager.SaveChangesAsync(IList`1 entriesToSave, CancellationToken cancellationToken)
         at Microsoft.EntityFrameworkCore.ChangeTracking.Internal.StateManager.SaveChangesAsync(StateManager stateManager, Boolean acceptAllChangesOnSuccess, CancellationToken cancellationToken)
         at Npgsql.EntityFrameworkCore.PostgreSQL.Storage.Internal.NpgsqlExecutionStrategy.ExecuteAsync[TState,TResult](TState state, Func`4 operation, Func`4 verifySucceeded, CancellationToken cancellationToken)
         at Microsoft.EntityFrameworkCore.DbContext.SaveChangesAsync(Boolean acceptAllChangesOnSuccess, CancellationToken cancellationToken)

Screenshots or Videos

No response

Additional Context

When a user is synchronized via SCIM, their external system ID is inserted in the field "ExternalId" in "OrganizationUser" table - which has a varchar(300) type.

When that same user tries to login via SSO for the virst time, the same system ID (in this case) is inserted in the field "ExternalId" in the "SSOUser" table - which has a varchar(50) type.

Can the width of SsoUser.ExternalId be increased to match OrganizationUser.ExternalId?

Githash Version

09f86f23-dirty (from self-hosted:dev test)

Environment Details

OS: Docker host is Ubuntu 22.04 Environment: Docker self-hosted:beta and :dev Hardware: VM on Epyc 7402 ~64GB allocated mem ~1TB allocated NVMe

Database Image

postgres:15

Issue-Link

https://github.com/bitwarden/server/issues/2480

Issue Tracking Info

sso-bitwarden commented 1 year ago

Hi there,

I am unable to reproduce this issue, the error message looks valid though. It has been escalated for further investigation. If you have more information that can help us, please add it below.

Thanks!

obrienmd commented 1 year ago

Are you able to determine the size of externalId in your SsoUser table in your repro case?

sso-bitwarden commented 1 year ago

Yes, the size is Varchar(50), but I didn't encounter the error when following your steps. Anyway, the Bitwarden Engineering team will check this report further.

obrienmd commented 1 year ago

Thanks much! Perhaps your test SSO provider uses a shorter ID than Authentik?

On Mon, May 1, 2023, 7:13 PM sso-bitwarden @.***> wrote:

Yes, the size is Varchar(50), but I didn't encounter the error when following your steps. Anyway, the Bitwarden Engineering team will check this report further.

— Reply to this email directly, view it on GitHub https://github.com/bitwarden/server/issues/2891#issuecomment-1530773072, or unsubscribe https://github.com/notifications/unsubscribe-auth/AAC2L56U7GSJNDFFUGRRBCTXEBUVZANCNFSM6AAAAAAXOQSD5I . You are receiving this because you authored the thread.Message ID: @.***>

sso-bitwarden commented 1 year ago

I tested using Okta. Yes, the ExternalID was pretty short. How long the ExternalID for Authentik?

obrienmd commented 1 year ago

64 chars

justindbaur commented 1 year ago

I am removing the unified label as the unified supported databases match the column length of our MS SQL database but I will escalate internally as to why this one is so much shorter than other ExternalId columns we have.

justindbaur commented 1 year ago

Welp quick discussion 😄 We are going to bump the column length. I'll put this on my backlog and update here when the work is merged.

obrienmd commented 1 year ago

Thanks!