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

FI-2857: Add auth input option #514

Merged
merged 34 commits into from
Aug 21, 2024
Merged

FI-2857: Add auth input option #514

merged 34 commits into from
Aug 21, 2024

Conversation

AlyssaWang
Copy link
Collaborator

@AlyssaWang AlyssaWang commented Jul 9, 2024

Summary

Creates auth info inputs for both auth and access modes.

Testing Guidance

Confirm that inputs work with the AuthInfo test suite. Check new dropdown and single checkbox input options for behavior issues.

Copy link

codecov bot commented Jul 9, 2024

Codecov Report

Attention: Patch coverage is 76.50177% with 266 lines in your changes missing coverage. Please review.

Project coverage is 79.72%. Comparing base (55dafb6) to head (3a55d12).
Report is 1 commits behind head on main.

Files Patch % Lines
client/src/components/InputsModal/InputFields.tsx 40.20% 49 Missing and 9 partials ⚠️
client/src/components/InputsModal/InputsModal.tsx 20.96% 45 Missing and 4 partials ⚠️
client/src/components/InputsModal/AuthSettings.ts 90.00% 15 Missing and 20 partials ⚠️
client/src/components/InputsModal/InputAccess.tsx 78.23% 13 Missing and 19 partials ⚠️
client/src/components/InputsModal/InputAuth.tsx 77.44% 10 Missing and 20 partials ⚠️
...src/components/InputsModal/InputSingleCheckbox.tsx 79.20% 15 Missing and 6 partials ⚠️
...lient/src/components/InputsModal/InputCombobox.tsx 82.02% 8 Missing and 8 partials ⚠️
...nt/src/components/InputsModal/AuthTypeSelector.tsx 88.23% 5 Missing and 3 partials ⚠️
...c/components/InputsModal/InputOAuthCredentials.tsx 70.00% 2 Missing and 4 partials ⚠️
...ent/src/components/InputsModal/InputRadioGroup.tsx 68.75% 4 Missing and 1 partial ⚠️
... and 4 more
Additional details and impacted files
@@            Coverage Diff             @@
##             main     #514      +/-   ##
==========================================
+ Coverage   79.69%   79.72%   +0.02%     
==========================================
  Files         243      249       +6     
  Lines       12178    13177     +999     
  Branches     1205     1290      +85     
==========================================
+ Hits         9705    10505     +800     
- Misses       1808     1922     +114     
- Partials      665      750      +85     
Flag Coverage Δ
backend 91.66% <ø> (ø)
frontend 74.64% <76.50%> (+0.65%) ⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

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

Copy link
Contributor

@vanessuniq vanessuniq left a comment

Choose a reason for hiding this comment

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

  • For auth and access mode : the auth_type value is not sent to the backend.
  • Auth mode: the auth_type dropdown does not behave properly when switching values. On change, the auth fields should also change to match the selected auth type.
  • Access Mode:
    • I think we also want to have the auth type dropdown here.
    • When refresh_token is present, all types display more fields than needed.

Copy link
Collaborator

@Jammjammjamm Jammjammjamm left a comment

Choose a reason for hiding this comment

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

I can't run the auth mode tests even though the inputs look complete. Also, an encryption algorithm should be selected by default.

Screenshot 2024-07-25 at 8 40 40 AM

@AlyssaWang
Copy link
Collaborator Author

Ready for rereview. My main remaining question is concerning some values and how to normalize the capitalization:

get and post vs GET and POST
s256 vs S256
es384 and rs384 vs ES384 and RS384

I've capitalized everything for now as I believe that's how they are in some of the non-dev tests but let me know if this should/will change.

Copy link
Contributor

@vanessuniq vanessuniq left a comment

Choose a reason for hiding this comment

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

Seams to work as expected:

  • each auth type shows the correct fields
  • switching auth type values dynamically changes the fields to display matching the auth type
  • all auth attributes are sent to the back end

Note: the input locked seems to be deactivated on the auth type dropdown
Screenshot 2024-08-09 at 10 28 50 AM
Screenshot 2024-08-09 at 10 29 21 AM

client/src/components/InputsModal/AuthTypeSelector.tsx Outdated Show resolved Hide resolved
Copy link
Collaborator

@Jammjammjamm Jammjammjamm left a comment

Choose a reason for hiding this comment

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

Backend Services Auth Mode input should not show:

  • Proof Key for Code Exchange (PKCE)
  • PKCE Code Challenge Method
  • Authorization Request Method

Backend Services Access Mode input should not show Refresh Token.

@AlyssaWang AlyssaWang merged commit c91b44c into main Aug 21, 2024
10 checks passed
@AlyssaWang AlyssaWang deleted the FI-2857-auth-input branch August 21, 2024 13:40
@rpassas rpassas mentioned this pull request Sep 11, 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