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

feat: backchannel request logging #3067

Merged
merged 2 commits into from
Apr 24, 2022

Conversation

aarmam
Copy link
Contributor

@aarmam aarmam commented Apr 13, 2022

This pull request adds minor logging improvements.

  • Successful back-channel logout request logging. This is useful for #2844 when trigger_backchannel_logout=true. Additionally log token for both success/failed responses.
  • ID token logging in POST /oauth2/token endpoint

Checklist

  • I have read the contributing guidelines.
  • I have referenced an issue containing the design document if my change
    introduces a new feature.
  • I am following the
    contributing code guidelines.
  • I have read the security policy.
  • I confirm that this pull request does not address a security
    vulnerability. If this pull request addresses a security. vulnerability, I
    confirm that I got green light (please contact
    security@ory.sh) from the maintainers to push
    the changes.
  • [] I have added tests that prove my fix is effective or that my feature
    works.
  • [] I have added or changed the documentation.

@aarmam aarmam requested a review from aeneasr as a code owner April 13, 2022 16:48
@codecov
Copy link

codecov bot commented Apr 13, 2022

Codecov Report

Merging #3067 (e799a62) into master (a3c4304) will increase coverage by 0.06%.
The diff coverage is 57.14%.

❗ Current head e799a62 differs from pull request most recent head 381638e. Consider uploading reports for the commit 381638e to get more accurate results

@@            Coverage Diff             @@
##           master    #3067      +/-   ##
==========================================
+ Coverage   79.57%   79.63%   +0.06%     
==========================================
  Files         112      112              
  Lines        7957     7956       -1     
==========================================
+ Hits         6332     6336       +4     
+ Misses       1223     1217       -6     
- Partials      402      403       +1     
Impacted Files Coverage Δ
consent/strategy_default.go 70.52% <57.14%> (+0.82%) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 222a01b...381638e. Read the comment docs.

@aarmam aarmam changed the title Feature/backchannel and id token logging feat: backchannel and id token logging Apr 13, 2022
@aarmam aarmam changed the title feat: backchannel and id token logging feat: backchannel request and id token logging Apr 13, 2022
@aarmam aarmam force-pushed the feature/backchannel-and-id-token-logging branch from 3e99599 to 227d053 Compare April 13, 2022 17:03
Copy link
Member

@aeneasr aeneasr left a comment

Choose a reason for hiding this comment

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

Awesome, thank you for your contribution! This looks pretty good and I have some ideas how to improve it further :)

oauth2/handler.go Outdated Show resolved Hide resolved
consent/strategy_default.go Outdated Show resolved Hide resolved
@aarmam aarmam force-pushed the feature/backchannel-and-id-token-logging branch 2 times, most recently from 621005e to 186e749 Compare April 19, 2022 08:23
@aarmam aarmam changed the title feat: backchannel request and id token logging feat: backchannel request logging Apr 19, 2022
@aarmam aarmam force-pushed the feature/backchannel-and-id-token-logging branch from 0f7ed3b to 381638e Compare April 19, 2022 12:33
@aarmam aarmam requested a review from aeneasr April 19, 2022 12:50
@aeneasr aeneasr merged commit 6dda48d into ory:master Apr 24, 2022
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.

2 participants