Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

PAS-536 | Add support for AuthenticatorDisplayName #713

Merged
merged 9 commits into from
Oct 1, 2024
Merged

Conversation

Tyrrrz
Copy link
Contributor

@Tyrrrz Tyrrrz commented Aug 30, 2024

Copy link
Member

@abergs abergs left a comment

Choose a reason for hiding this comment

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

We need to add migrations, corect?

@Tyrrrz
Copy link
Contributor Author

Tyrrrz commented Sep 2, 2024

@abergs yes, correct

@Tyrrrz Tyrrrz marked this pull request as ready for review September 2, 2024 18:54
@Tyrrrz Tyrrrz requested a review from a team as a code owner September 2, 2024 18:54
@Tyrrrz Tyrrrz enabled auto-merge (squash) September 2, 2024 18:54
@Tyrrrz Tyrrrz disabled auto-merge September 2, 2024 18:55
@Tyrrrz
Copy link
Contributor Author

Tyrrrz commented Sep 2, 2024

This needs a new release of Passwordless.NET and a package update to resolve missing references

@Tyrrrz Tyrrrz marked this pull request as draft September 2, 2024 18:55
@Tyrrrz
Copy link
Contributor Author

Tyrrrz commented Sep 9, 2024

Added migrations and updated the Passwordless.NET SDK version to 2.1.0-beta.3 which is pending approval here: https://github.com/bitwarden/passwordless-devops/actions/runs/10775849985

Once that package is released, this PR should technically be good to merge. Would still need to run some smoke tests for good measure, however.

@Tyrrrz Tyrrrz marked this pull request as ready for review September 9, 2024 21:23
@@ -16,53 +16,32 @@

namespace Passwordless.Service;

public class Fido2Service : IFido2Service
public class Fido2Service(
Copy link
Contributor Author

@Tyrrrz Tyrrrz Sep 9, 2024

Choose a reason for hiding this comment

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

Only formatting changes here except line 251

@Tyrrrz
Copy link
Contributor Author

Tyrrrz commented Sep 9, 2024

Should be ready

@Tyrrrz Tyrrrz enabled auto-merge (squash) September 9, 2024 21:27
Copy link

codecov bot commented Sep 9, 2024

Codecov Report

Attention: Patch coverage is 52.53785% with 533 lines in your changes missing coverage. Please review.

Project coverage is 35.56%. Comparing base (3cfa4f5) to head (91e50fc).
Report is 15 commits behind head on main.

Files with missing lines Patch % Lines
...0240909154138_AuthenticatorDisplayName.Designer.cs 0.00% 459 Missing ⚠️
src/Service/Fido2Service.cs 72.91% 24 Missing and 2 partials ⚠️
...dminConsole/Components/Shared/Credentials.razor.cs 0.00% 25 Missing ⚠️
.../Sqlite/20240909154138_AuthenticatorDisplayName.cs 0.00% 10 Missing ⚠️
...s/Mssql/20240909154123_AuthenticatorDisplayName.cs 60.00% 4 Missing ⚠️
...vice/Migrations/Mssql/MsSqlContextModelSnapshot.cs 0.00% 4 Missing ⚠️
...ce/Migrations/Sqlite/SqliteContextModelSnapshot.cs 0.00% 4 Missing ⚠️
src/AdminConsole/Helpers/StringExtensions.cs 0.00% 1 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main     #713      +/-   ##
==========================================
+ Coverage   35.15%   35.56%   +0.40%     
==========================================
  Files         579      584       +5     
  Lines       31252    32202     +950     
  Branches      949      950       +1     
==========================================
+ Hits        10988    11452     +464     
- Misses      20119    20603     +484     
- Partials      145      147       +2     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@Tyrrrz Tyrrrz merged commit b7165e0 into main Oct 1, 2024
16 checks passed
@Tyrrrz Tyrrrz deleted the auth-display-name branch October 1, 2024 14:30
Tyrrrz added a commit that referenced this pull request Oct 2, 2024
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