Skip to content

Conversation

@thruflo
Copy link
Contributor

@thruflo thruflo commented Sep 21, 2013

May generally be useful to know information about the unauthorized request when handling it.

My use case is picking out pre-filled-in login/signup data from the request data/params.

@eddiemonge
Copy link

why not pass in the full response as well?

@thruflo
Copy link
Contributor Author

thruflo commented Oct 17, 2013

Any further thoughts on this? I'm about to build an application using it, so be great to know if it's a bad idea for some reason...

@witoldsz
Copy link
Owner

Well, the thing is I don't get it:

My use case is picking out pre-filled-in login/signup data from the request data/params.

Why do you want my module to broadcast pre-filled-in data? Why won't you handle it separately from 401 interceptor? If you are submitting some a login form then you can handle the used login/pass yourself, why to mix the interceptor in?

@thruflo
Copy link
Contributor Author

thruflo commented Oct 25, 2013

Hey,

My use case is not something I'd imagine is very common. I'm building an app that uses gradual engagement and, in my case, I've crafted a flow which attempts implicit authentication, using params picked up from requests that came back 401, before showing the signup/login form.

A more common use case might be to display an error message -- why the original request came back 401 -- which would require having access to the response.

I guess my question was more along the lines of "is this insecure" but in hindsight, I can't see how broadcasting the response can be a problem.

James.

witoldsz added a commit that referenced this pull request Oct 28, 2013
interceptor: broadcast `response` with `event:auth-loginRequired` message.

See: #41
@witoldsz witoldsz merged commit fda0e07 into witoldsz:master Oct 28, 2013
@witoldsz
Copy link
Owner

Does not look like it can hurt anyone, so merging.

@eddiemonge
Copy link

yay!

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.

3 participants