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.

Sign up for GitHub

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

Jump to bottom

Use session_state to backup consumer state if available #854

Merged
merged 1 commit into from
Apr 5, 2023
Merged

Use session_state to backup consumer state if available #854

merged 1 commit into from
Apr 5, 2023

Conversation

gbip
Copy link
Contributor

@gbip gbip commented Apr 3, 2023 •
edited
Loading

This PR improves the support for Open ID Connect session management.

When processing the Authorization Response from an Authorization Server, it allows library users to save the consumer data using the session_state parameter from the response.

This makes it easier to integrate this library in a webserver, as session keys are very usefull to find back the appropriate Consumer instance from the SessionBackend.

More details on the motivation behind this PR can be found in the following discussion : #853

  • Any changes relevant to users are recorded in the CHANGELOG.md.
  • The documentation has been updated, if necessary.
  • New code is annotated.
  • Changes are covered by tests.

@gbip gbip mentioned this pull request Apr 3, 2023
Closed
@gbip gbip changed the title Draft : Use session_state to backup consumer state if available Use session_state to backup consumer state if available Apr 4, 2023
@gbip gbip marked this pull request as ready for review April 4, 2023 11:39
@gbip
Copy link
Contributor Author

gbip commented Apr 4, 2023

@tpazderka : Can you review this Pull Request please ?

tpazderka
tpazderka reviewed Apr 4, 2023
View reviewed changes
src/oic/oic/consumer.py Outdated Show resolved Hide resolved
@gbip
Copy link
Contributor Author

gbip commented Apr 4, 2023

I'm not sure what the CI issue is :

 quality: commands[7]> twine check 'dist/*'
Checking dist/oic-1.5.0.tar.gz: PASSED
.pkg: _exit> python /opt/hostedtoolcache/Python/3.7.16/x64/lib/python3.7/site-packages/pyproject_api/_backend.py True setuptools.build_meta __legacy__
  py37: OK (44.91=setup[13.75]+cmd[31.16] seconds)
  py38: SKIP (0.32 seconds)
  py39: SKIP (0.01 seconds)
  py310: OK (37.38=setup[9.58]+cmd[27.81] seconds)
  py311: SKIP (0.00 seconds)
  docs: OK (14.64=setup[12.71]+cmd[1.92] seconds)
  quality: FAIL code 1 (41.35=setup[14.46]+cmd[0.53,6.32,4.21,7.00,2.88,0.52,5.06,0.37] seconds)

@schlenk
Copy link
Collaborator

schlenk commented Apr 4, 2023 

quality: commands[1]> pylama src/ tests/
src/oic/oic/consumer.py:473:121 E501 line too long (130 > 120 characters) [pycodestyle]

Pylama complains about line length.

@codecov-commenter
Copy link

codecov-commenter commented Apr 5, 2023 •
edited
Loading

Codecov Report

Patch coverage: 100.00% and no project coverage change.

Comparison is base (3b1a9c3) 63.82% compared to head (433017c) 63.83%.

📣 This organization is not using Codecov’s GitHub App Integration. We recommend you install it so Codecov can continue to function properly for your repositories. Learn more

Additional details and impacted files 
@@           Coverage Diff           @@
##           master     #854   +/-   ##
=======================================
  Coverage   63.82%   63.83%           
=======================================
  Files          64       64           
  Lines       11877    11879    +2     
  Branches     2154     2155    +1     
=======================================
+ Hits         7581     7583    +2     
  Misses       3693     3693           
  Partials      603      603
Impacted Files Coverage Δ
src/oic/oic/consumer.py 80.07% <100.00%> (+0.14%) ⬆️

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

☔ View full report in Codecov by Sentry.
📢 Do you have feedback about the report comment? Let us know in this issue.

@tpazderka
Copy link
Collaborator

@schlenk Any objections?

@schlenk
Copy link
Collaborator

schlenk commented Apr 5, 2023

Looks good to me.
The "state" should be as opaque to the backend as a "sessionstate", so there is no real issue that the sdb backend could have troubles with the value.

@tpazderka tpazderka merged commit 444bd68 into CZ-NIC:master Apr 5, 2023
infohash added a commit to infohash/pyoidc that referenced this pull request Jul 2, 2023
@infohash
Squashed commit of the following:
a1829af 
commit 5245dca
Author: tpazderka <tomas.pazderka@nic.cz>
Date:   Tue May 16 19:48:56 2023 +0200

    Fix building of the documentation (CZ-NIC#860)

commit ba65e00
Author: Tomáš Pazderka <tomas.pazderka@nic.cz>
Date:   Mon May 15 09:01:02 2023 +0200

    Prepare 1.6.0 release

commit cfd0b6e
Author: Michael Schlenker <MichaelSchlenker@gmx.net>
Date:   Tue May 2 14:02:37 2023 +0200

    Repair some oauth_examples and get rid of the outdated jquery file (CZ-NIC#857)

    * More cleanup

    * Add oauth_example to quality checks

    Signed-off-by: Michael Schlenker <michael.schlenker@contact-software.com>

    * Update CHANGELOG.md

    ---------

    Signed-off-by: Michael Schlenker <michael.schlenker@contact-software.com>
    Co-authored-by: Michael Schlenker <michael.schlenker@contact-software.com>

commit 444bd68
Merge: 3b1a9c3 433017c
Author: tpazderka <tomas.pazderka@nic.cz>
Date:   Wed Apr 5 13:59:40 2023 +0200

    Merge pull request CZ-NIC#854 from gbip/853_session_state

    Use session_state to backup consumer state if available

commit 433017c
Author: Paul Florence <paul.florence@makina-corpus.com>
Date:   Mon Apr 3 17:37:30 2023 +0200

    Use 'session_state' from an Authentication Response to backup consumer state (if available)

commit 3b1a9c3
Merge: a8454e3 1d46f09
Author: tpazderka <tomas.pazderka@nic.cz>
Date:   Mon Mar 27 09:24:34 2023 +0200

    Merge pull request CZ-NIC#852 from CZ-NIC/851-add-consumer-complete-authn-method

    Add authn_method to Consumer.complete()

commit 1d46f09
Author: Jan Musílek <jan.musilek@nic.cz>
Date:   Thu Mar 23 16:25:48 2023 +0100

    Add authn_method to Consumer.complete()

commit a8454e3
Merge: 559c9c8 a855642
Author: tpazderka <tomas.pazderka@nic.cz>
Date:   Fri Mar 17 13:16:25 2023 +0100

    Merge pull request CZ-NIC#850 from hlin/master

    Correct OpenID Connect Core specification URL in doc

commit a855642
Author: Hypo Lin <hlin@hypo.name>
Date:   Fri Mar 17 18:56:19 2023 +0800

    Correct OpenID Connect Core specification URL in doc

commit 559c9c8
Merge: 76debd6 9670f83
Author: tpazderka <tomas.pazderka@nic.cz>
Date:   Mon Mar 13 16:06:25 2023 +0100

    Merge pull request CZ-NIC#847 from CZ-NIC/use-pydantic

    Improve settings by using pydantic

commit 9670f83
Author: Tomáš Pazderka <tomas.pazderka@nic.cz>
Date:   Mon Feb 20 12:42:46 2023 +0100

    Improve settings by using pydantic

commit 76debd6
Merge: 6f7f893 94d1f99
Author: tpazderka <tomas.pazderka@nic.cz>
Date:   Mon Mar 13 14:54:53 2023 +0100

    Merge pull request CZ-NIC#848 from CZ-NIC/bandit-timeouts

    Pass timeout to all requests

commit 94d1f99
Author: Tomáš Pazderka <tomas.pazderka@nic.cz>
Date:   Fri Mar 10 10:19:00 2023 +0100

    Pass timeout to all requests

commit 6f7f893
Merge: f6c590c 12247bc
Author: tpazderka <tomas.pazderka@nic.cz>
Date:   Mon Feb 20 09:38:17 2023 +0100

    Merge pull request CZ-NIC#846 from CZ-NIC/repo-move

    Fix links after repo move

commit 12247bc
Author: Tomáš Pazderka <tomas.pazderka@nic.cz>
Date:   Fri Feb 17 11:50:41 2023 +0100

    Fix links after repo move

    Close CZ-NIC#845
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@tpazderka tpazderka tpazderka approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@gbip @schlenk @codecov-commenter @tpazderka