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

Are we expected to call nonceStore.setUsed manually? #14

Closed
scaleupcto opened this issue Aug 12, 2014 · 1 comment
Closed

Are we expected to call nonceStore.setUsed manually? #14

scaleupcto opened this issue Aug 12, 2014 · 1 comment

Comments

@scaleupcto
Copy link

...from within provider.validate_request? It might make more sense to call it from within there, i.e. just inside the passed next function for this.nonceStore.isNew in Provider.prototype._valid_oauth

If we're expected to call it from within our own implementation of isNew itself, there's little value in documenting that it must be exposed externally, instead part of the contract of isNew should be that on subsequent calls for the same value it should return false.

@omsmith
Copy link
Owner

omsmith commented Aug 23, 2014

Once again, sorry for the delay.

If you're implementing your own NonceStore, then I suppose at the moment you would be expected to call it, yeah.

It would really make more sense to me to turn the NonceStore class into an abstract implementation, which provides sane defaults, and simply calls the _isSeen and _setUsed implementations of the extended classes.

This is maybe something we can do with a version bump - I had also started putting together a different library which is arranged more in the style I would have used if I initially authored this one (omsmith/node-lti-provider) - we'll see.

@omsmith omsmith closed this as completed Sep 19, 2014
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

No branches or pull requests

2 participants