Skip to content

add session_not_on_or_after #206

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

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

pcaskey
Copy link

@pcaskey pcaskey commented Mar 7, 2019

Sets the session_not_on_or_after parameter in downstream assertions
when appropriately configured.

All Submissions:

  • Have you checked to ensure there aren't other open Pull Requests for the same update/change?
  • Have you added an explanation of what problem you are trying to solve with this PR?
  • Have you added information on what your changes do and why you chose this as your solution?
  • Have you written new tests for your changes?
  • Does your submission pass tests?
  • This project follows PEP8 style guide. Have you run your code against the 'flake8' linter?

Sets the session_not_on_or_after parameter in downstream assertions
when appropriately configured.
@c00kiemon5ter c00kiemon5ter mentioned this pull request Apr 2, 2019
6 tasks
@c00kiemon5ter
Copy link
Member

c00kiemon5ter commented Apr 2, 2019

As mentioned on #207, you use a configuration option that is not declared anywhere, it is searched in the part of the configuration that belongs to pysaml2 but it is handled by satosa. This needs to be fixed.

Moreover, I'd like to find time to rework IdentityPython/pysaml2#518 so that we can have a base upon which we handle date/time. This could then be expressed by utilizing that functionality.

I'll keep this open in order to come back to it.

@pcaskey
Copy link
Author

pcaskey commented Apr 2, 2019 via email

@c00kiemon5ter
Copy link
Member

Unfortunately, I’m not sure how to fix the undeclared config option(s). Where would I do that?

Yep, this is common, and we must handle it in a way that is explicit what is what.

So, for example, I'm looking into the saml-backend configuration. The configuration has three parts:

  • The base:
module: satosa.backends.saml2.SAMLBackend
name: Saml2
config:
[...]

You can think of this as "metadata" for the file. They are a fixed set of options that describe who is going to load and handle this file, what name it has, and what is the configuration that will be provided.

  • The config for the module:
[...]
config:
  idp_blacklist_file: /path/to/blacklist.json

  # disco_srv must be defined if there is more than one IdP in the metadata specified above
  disco_srv: http://disco.example.com

  sp_config:
[...]

All options under config will be read by satosa. These options should change the way satosa works on top of pysaml2. sp_config and idp_config will be passed to pysaml2 directly - satosa doesn't really care about those options.

  • The pysaml2 config
  sp_config:
    key_file: backend.key
    cert_file: backend.crt
    organization: {display_name: Example Identities, name: Example Identities Org., url: 'http://www.example.com'}
    contact_person:
    - {contact_type: technical, email_address: technical@example.com, given_name: Technical}
[...]

anything under sp_config and idp_config is handled by pysaml2; it is loaded and given directly to pysaml2 handlers. Those settings are used by pysaml2 to load an IdP (frontend) or an SP (backend). They are defined by pysaml2 and handled by pysaml2.


There is blurry line between what should be part of pysaml2 and what should be part of satosa. I am very skeptical of whether pysaml2 should be including runnable services, like an IdP, or whether it should be just a library with behaviour decided and given by its users (ie satosa).
For now, anything that affects a single entity (an SP or IdP) and not the internal transformation of the data given and received, should go to pysaml2.

What you do here though, is passing through an option to a pysaml2 utility, which I'd say is fine to do. What needs to change is either to move the option outside the pysaml2 config, or look into pysaml2 and see whether such functionality is already there and make use of it.

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