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

share set token method #186

Merged
merged 1 commit into from
Nov 20, 2014
Merged

share set token method #186

merged 1 commit into from
Nov 20, 2014

Conversation

Blaze34
Copy link
Contributor

@Blaze34 Blaze34 commented Nov 11, 2014

No description provided.

@sahat
Copy link
Owner

sahat commented Nov 11, 2014

What's a use case for this?

It is meant to be an internal method.

@Blaze34
Copy link
Contributor Author

Blaze34 commented Nov 11, 2014

For example:
I have fast registration (only email require) and confirmation by email.
On sign up - I stay anonymous, but user was created.
I come by link from email and fill some additional information (name and password) and on success should be logged in

@sahat
Copy link
Owner

sahat commented Nov 12, 2014

What exactly are you trying to save? An access token? Because setToken refers to JSON Web Token and you can see where things get very confusing. The setToken should probably called saveToken or saveTokenToLocalStorage and rethink how setToken should behave.

@sars
Copy link

sars commented Nov 12, 2014

+1

There is only one way to save token now - to use $auth.login
But sometimes I would like to set token and be logged in when I request some other actions.

For example I have other registration flow: user just put email, then receive email confirmation then go to the confirmation page and complete registration.

So when user complete registration (it is now sign up already) server return auth token, that I want to save to be logged in.

@Blaze34
Copy link
Contributor Author

Blaze34 commented Nov 12, 2014

I want to have opportunity authorize user anywhere from any response without additional requests.

As in your code private method shared.getToken() accesed by public $auth.getToken(), I want open access to private method shared.setToken({access_token: token}, isLinking) through public method $auth.setToken(token, isLinking).

In this case:

  • private is method inside satellizer, which I couldn't call
  • public is API Reference, which I could call

@sars
Copy link

sars commented Nov 13, 2014

@sahat what do you think?

@tdavis
Copy link

tdavis commented Nov 19, 2014

Going to throw my hat in this ring. I have a need a need for speed for setToken as well; or, more precisely, I want to extend the token's payload with other key/val pairs. I have written a method for this purpose:

  extendTokenParams: function(obj) {
    if (!this.isAuthenticated()) {
      return;
    }

    var token = $auth.getToken();
    var payload = _.extend($auth.getPayload() || {}, obj);
    var parts = token.split('.');

    parts[1] = $window.btoa(JSON.stringify(payload));
    $auth.setToken(parts.join('.'));
  }

Unfortunately, $auth.setToken() only exists in my head...

@sahat sahat merged commit 28d017b into sahat:master Nov 20, 2014
@sahat
Copy link
Owner

sahat commented Nov 20, 2014

I went ahead and merged this PR, meanwhile if you have suggestions to improve it please feel free to chime in. The only thing I don't like about this change is that access_token is hard coded. Doesn't that mean it will only be useful for implicit grant oauth 2.0?

@tdavis
Copy link

tdavis commented Nov 20, 2014

Not really, no. @sars wrote it that way because using the existing shared.setToken() is something of a hack as it is designed as an XHR callback, not a normal method. I would have refactored things a bit rather than just fake a response to the callback. I also feel like exposing the raw setToken(), while the obvious choice, probably isn't what most people actually want either; does anyone really want to parse, decode, and encode the token just to add something to the payload?

In the case of "I want to require email confirmation," it would likely be more maintainable to simply authorize the user but indicate their email confirmation state in the payload, rather than craft your own temporary token. That's kinda the point of the payload: adding extra non-secret information that can be validated by client and server.

@tdavis
Copy link

tdavis commented Nov 20, 2014

(Although, @sahat, you are correct that the public setToken() implicitly relies on an implementation detail of the private setToken(), which would lead to regression in the case that the private method starts behaving differently for properties other than access_token)

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.

4 participants