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

/system/saml/metadata serves potentially outdated data #165

Open
peb-adr opened this issue Oct 24, 2023 · 2 comments
Open

/system/saml/metadata serves potentially outdated data #165

peb-adr opened this issue Oct 24, 2023 · 2 comments
Labels
Milestone

Comments

@peb-adr
Copy link
Member

peb-adr commented Oct 24, 2023

In the current SAML implementation the datastore is only queried once for the organization.saml_metadata_sp field which is then stored in a local variable and served on /system/saml/metadata as long as the container lives - regardless of changes to the datastore.

A solution for this needs to be found. Options discussed before are:

  • The robust solution would be to have the auth service listen to datastore-events changing the field in one or another way. I.e. either making a subscription to the autoupdate service or by attaching to the redis message bus itself.
    However this would be quite some overhead, since the auth service otherwise doesn't listen for changed data, so an adapter would have to be implemented solely for this use-case.
  • Polling. The auth service could regularly poll the field from the datastore to get potentially updated data.
    Obviously this implicates a tradeoff between the delay for the served metadata to be up-to-date vs. how often it will be re-polled unnecessarily.
  • Not caching in a local variable, i.e. for every request to /system/saml/metadata/ the datastore is queried for the current metadata.
    This would also mean alot of avoidable requests / load, albeit probably very low performance impact.

As of right now the container needs to be restarted after changing metadata in order for it to be served on /system/saml/metadata.

@peb-adr peb-adr added the bug label Oct 24, 2023
@jsangmeister jsangmeister added this to the 4.2 milestone Dec 12, 2023
@jsangmeister
Copy link
Contributor

Another problem/overhead with your first two solutions is that the service currently does not have a long-running thread which could do the polling/listening, so this would have to be additionally implemented.

Not caching in a local variable, i.e. for every request to /system/saml/metadata/ the datastore is queried for the current metadata.
This would also mean alot of avoidable requests / load, albeit probably very low performance impact.

Do we have any statistics on how often such a request is done? If it's a reasonable amount, I would favor this solution, as it's the easiest to implement.

@peb-adr
Copy link
Member Author

peb-adr commented Jan 16, 2024

  • The robust solution would be to have the auth service listen to datastore-events changing the field in one or another way. I.e. either making a subscription to the autoupdate service or by attaching to the redis message bus itself.
    However this would be quite some overhead, since the auth service otherwise doesn't listen for changed data, so an adapter would have to be implemented solely for this use-case.

Talked about this issue for a few minutes in plenum. This was the favored solution, as it will yield most consistent results.
We would want to subscribe to the autoupdate for this. Apparently there is an interface to do this internally without user authentication.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

2 participants