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

FAPI2 profile support #317

Merged
merged 11 commits into from
Dec 27, 2023
Merged

FAPI2 profile support #317

merged 11 commits into from
Dec 27, 2023

Conversation

paulswartz
Copy link
Collaborator

@paulswartz paulswartz commented Dec 23, 2023

Ref: #316

Open Questions

  • should this split the profile atoms into fapi2_security_profile and fapi2_message_signing? The latter seems to also require request objects and JARM (even if the conformance suite doesn't seem to).

TODO

  • test coverage
  • documentation
  • Elixir implementation

@coveralls
Copy link

coveralls commented Dec 24, 2023

Pull Request Test Coverage Report for Build 132

  • 100 of 105 (95.24%) changed or added relevant lines in 5 files are covered.
  • 1 unchanged line in 1 file lost coverage.
  • Overall coverage increased (+0.8%) to 93.994%

Changes Missing Coverage Covered Lines Changed/Added Lines %
src/oidcc_profile.erl 44 45 97.78%
src/oidcc_token.erl 13 14 92.86%
src/oidcc_authorization.erl 18 21 85.71%
Files with Coverage Reduction New Missed Lines %
src/oidcc_token.erl 1 88.46%
Totals Coverage Status
Change from base Build 142: 0.8%
Covered Lines: 892
Relevant Lines: 949

💛 - Coveralls

@paulswartz paulswartz marked this pull request as ready for review December 26, 2023 20:37
@maennchen
Copy link
Member

should this split the profile atoms into fapi2_security_profile and fapi2_message_signing?

That would probably be good. FAPI1 also has very different requirements between the specs.

src/oidcc_profile.erl Outdated Show resolved Hide resolved
@maennchen
Copy link
Member

maennchen commented Dec 26, 2023

@paulswartz mTLS should be relatively simple to handle:

The user can already supply ssl options. If you include cert[file], key[file] and verify: :verify_peer, mTLS is enabled.

So we can just verify that they either are set, tls_client_auth is supported by the provider and otherwise require DPoP.

@maennchen maennchen changed the title wip: FAPI2 profile support FAPI2 profile support Dec 26, 2023
lib/oidcc/client_context.ex Outdated Show resolved Hide resolved
@maennchen
Copy link
Member

After this minor correction good for merge.

@maennchen
Copy link
Member

@paulswartz Can you open issues for all the TODOs that are missing for full compliance? (Like JARM and mTLS)

Co-authored-by: Jonatan Männchen <[email protected]>
maennchen
maennchen previously approved these changes Dec 27, 2023
@maennchen
Copy link
Member

@paulswartz Can you run mix format so that I can merge?

@maennchen maennchen merged commit 108996f into erlef:main Dec 27, 2023
25 checks passed
@paulswartz paulswartz deleted the profiles-fapi2 branch January 1, 2024 21:45
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants