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

[ENHANCE] Ability to generate your own Request ID #659

Closed
keyCat opened this issue Nov 30, 2021 · 4 comments
Closed

[ENHANCE] Ability to generate your own Request ID #659

keyCat opened this issue Nov 30, 2021 · 4 comments

Comments

@keyCat
Copy link

keyCat commented Nov 30, 2021

Hello. I am not sure if this request is more related to passport-saml or node-saml package, but probably to both.
I've encountered a use-case where I'd like to be able to generate my own unique request ID.

Is your feature request related to a problem? Please describe.
I would like to be able to store associated data using a Request ID, so later I can retrieve this data from my own store during the callback stage using the ID from InResponseTo field.

Describe the solution you'd like
The easiest approach to a solution (as I see it) is to allow to define your own generateUniqueID() (or even generateUniqueIDAsync()) functions via provider options.

I am using MultiSamlStrategy, so my provider config is generated per request with getSamlOptions(), thus making the use of potential generateUniqueID() possible, but I see how this option would not be very useful during a pre-configured single Strategy use.

I will gladly accept any advice on how to solve this issue any other way, if such a way exists.

Thanks!

UPD: Upon further inspection, I've found out that this was already implemented in node-saml/node-saml@9081e89 but that version of it isn't used by passport-saml yet.

@cjbarth
Copy link
Collaborator

cjbarth commented Nov 30, 2021

Do you mean something like this? node-saml/node-saml#30

@keyCat
Copy link
Author

keyCat commented Nov 30, 2021

@cjbarth Yes! I didn't notice initially that this was already in the works. Thanks!

@cjbarth
Copy link
Collaborator

cjbarth commented Nov 30, 2021

Feel free to put up a PR with documentation on how to use it.

@cjbarth cjbarth closed this as completed Nov 30, 2021
@srd90
Copy link

srd90 commented Dec 12, 2021

@keyCat wrote:

I would like to be able to store associated data using a Request ID, so later I can retrieve this data from my own store during the callback stage using the ID from InResponseTo field.

SAML standard has RelayState for that kind of use cases. See more e.g. from: #541 (comment)

Furthermore it seems that passport-saml's RelayState support has had updates recently (see discussion from issue #157 ).

If you (@keyCat) choose to solve your issue with custom request IDs and if you introduce PR suggested by @cjbarth consider also documenting when you should use request ID approach instead of what SAML standard provides otherwise there is a risk that request ID approach (hack) is used by those who do not understand implications of e.g. non-random request IDs etc.

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

No branches or pull requests

3 participants