Skip to content

[PM-3263] fix identity server tests for passkey registration#3331

Merged
ike-kottlowski merged 2 commits into
fido2from
auth/PM-3263/fix-identity-server-tests-for-passkey-registration
Oct 12, 2023
Merged

[PM-3263] fix identity server tests for passkey registration#3331
ike-kottlowski merged 2 commits into
fido2from
auth/PM-3263/fix-identity-server-tests-for-passkey-registration

Conversation

@ike-kottlowski

@ike-kottlowski ike-kottlowski commented Oct 6, 2023

Copy link
Copy Markdown
Contributor

Type of change

  • Bug fix
  • New feature development
  • Tech debt (refactoring, code cleanup, dependency upgrades, etc)
  • Build/deploy pipeline (DevOps)
  • Other

Objective

Some Tests were failing after the addition of the WebAuthnCredentialRepository. This addresses those test failures.

Code changes

  • src\Infrastructure.EntityFramework\EntityFrameworkServiceCollectionExtensions.cs: Added the WebAuthnCredentialRepository to the DI.
  • Identity.IntegrationTest\openid-configuration.json: Added extension to the "grant_types_supported".

Before you submit

  • Please check for formatting errors (dotnet format --verify-no-changes) (required)
  • If making database changes - make sure you also update Entity Framework queries and/or migrations
  • Please add unit tests where it makes sense to do so (encouraged but not required)
  • If this change requires a documentation update - notify the documentation team
  • If this change has particular deployment requirements - notify the DevOps team

@ike-kottlowski ike-kottlowski requested review from a team as code owners October 6, 2023 19:16
@ike-kottlowski ike-kottlowski changed the base branch from master to fido2 October 6, 2023 19:17
@ike-kottlowski ike-kottlowski marked this pull request as draft October 6, 2023 19:25
@bitwarden-bot

Copy link
Copy Markdown
Collaborator

Logo
Checkmarx One – Scan Summary & Details2064a3d6-835b-46d2-87ea-896803816272

New Issues

Severity Issue Source File / Package Checkmarx Insight
MEDIUM CSRF /src/Api/Auth/Controllers/WebAuthnController.cs: 79 Attack Vector
MEDIUM CSRF /src/Api/Auth/Controllers/WebAuthnController.cs: 62 Attack Vector
MEDIUM CSRF /src/Api/Auth/Controllers/WebAuthnController.cs: 37 Attack Vector
LOW Heap_Inspection /src/Core/Constants.cs: 39 Attack Vector
LOW Log_Forging /src/Api/Auth/Controllers/WebAuthnController.cs: 62 Attack Vector
LOW Log_Forging /src/Api/Auth/Controllers/WebAuthnController.cs: 62 Attack Vector
LOW Use_Of_Hardcoded_Password /src/Core/Constants.cs: 39 Attack Vector

@ike-kottlowski ike-kottlowski marked this pull request as ready for review October 10, 2023 21:02
"password",
"urn:ietf:params:oauth:grant-type:device_code"
"urn:ietf:params:oauth:grant-type:device_code",
"extension"

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

note: extension will most likely change name to webauthn as part of PM-2032 Build server endpoints to support authentication with a passkey

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yep, but to avoid scope creeeeeeep I opted to get the test to pass and address the new name in the appropriate JIRA task.

@ike-kottlowski ike-kottlowski merged commit 3bda447 into fido2 Oct 12, 2023
@ike-kottlowski ike-kottlowski deleted the auth/PM-3263/fix-identity-server-tests-for-passkey-registration branch October 12, 2023 20:09
trmartin4 added a commit that referenced this pull request Oct 30, 2023
* support for fido2 auth

* stub out registration implementations

* stub out assertion steps and token issuance

* verify token

* webauthn tokenable

* remove duplicate expiration set

* revert sqlproj changes

* update sqlproj target framework

* update new validator signature

* [PM-2014] Passkey registration (#2915)

* [PM-2014] chore: rename `IWebAuthnRespository` to `IWebAuthnCredentialRepository`

* [PM-2014] fix: add missing service registration

* [PM-2014] feat: add user verification when fetching options

* [PM-2014] feat: create migration script for mssql

* [PM-2014] chore: append to todo comment

* [PM-2014] feat: add support for creation token

* [PM-2014] feat: implement credential saving

* [PM-2014] chore: add resident key TODO comment

* [PM-2014] feat: implement passkey listing

* [PM-2014] feat: implement deletion without user verification

* [PM-2014] feat: add user verification to delete

* [PM-2014] feat: implement passkey limit

* [PM-2014] chore: clean up todo comments

* [PM-2014] fix: add missing sql scripts

Missed staging them when commiting

* [PM-2014] feat: include options response model in swagger docs

* [PM-2014] chore: move properties after ctor

* [PM-2014] feat: use `Guid` directly as input paramter

* [PM-2014] feat: use nullable guid in token

* [PM-2014] chore: add new-line

* [PM-2014] feat: add support for feature flag

* [PM-2014] feat: start adding controller tests

* [PM-2014] feat: add user verification test

* [PM-2014] feat: add controller tests for token interaction

* [PM-2014] feat: add tokenable tests

* [PM-2014] chore: clean up commented premium check

* [PM-2014] feat: add user service test for credential limit

* [PM-2014] fix: run `dotnet format`

* [PM-2014] chore: remove trailing comma

* [PM-2014] chore: add `Async` suffix

* [PM-2014] chore: move delay to constant

* [PM-2014] chore: change `default` to `null`

* [PM-2014] chore: remove autogenerated weirdness

* [PM-2014] fix: lint

* Added check for PasswordlessLogin feature flag on new controller and methods. (#3284)

* Added check for PasswordlessLogin feature flag on new controller and methods.

* fix: build error from missing constructor argument

---------

Co-authored-by: Andreas Coroiu <andreas.coroiu@gmail.com>

* [PM-4171] Update DB to support PRF (#3321)

* [PM-4171] feat: update database to support PRF

* [PM-4171] feat: rename `DescriptorId` to `CredentialId`

* [PM-4171] feat: add PRF felds to domain object

* [PM-4171] feat: add `SupportsPrf` column

* [PM-4171] fix: add missing comma

* [PM-4171] fix: add comma

* [PM-3263] fix identity server tests for passkey registration (#3331)

* Added WebAuthnRepo to EF DI

* updated config to match current grant types

* Remove ExtensionGrantValidator (#3363)

* Linting

---------

Co-authored-by: Andreas Coroiu <acoroiu@bitwarden.com>
Co-authored-by: Andreas Coroiu <andreas.coroiu@gmail.com>
Co-authored-by: Todd Martin <106564991+trmartin4@users.noreply.github.com>
Co-authored-by: Ike <137194738+ike-kottlowski@users.noreply.github.com>
Co-authored-by: Todd Martin <tmartin@bitwarden.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants