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

Allow modification of the message after successful authorization #85

Closed
stanislavHamara opened this issue Oct 9, 2018 · 7 comments
Closed
Labels

Comments

@stanislavHamara
Copy link

Expected Behavior

The message that is shown to the user after they succesfully log in should be modifiable.

Describe the problem

When the user successfully logs in, they are presented with a tab that says "Close your browser to continue". This is most of the times not the right call to action as the AppAuth does not open a tab in a new window if a browser is already open.

Especially when being used with an electron app, it would be useful from a UX point of view if we could use a custom message once the user logs in (or at least be able to change it to "Close the tab to continue").

Steps to reproduce the behavior

Succesfully log in in the browser.

Environment

  • AppAuth-JS version: 0.3.5
  • AppAuth-JS Environment (Node, Browser (UserAgent), ...): Node
  • Source code snippts (inline or JSBin)
@tikurahul
Copy link
Collaborator

You can customize node_request_handler.ts to change the message already.
https://github.com/openid/AppAuth-JS/blob/master/src/node_support/node_request_handler.ts#L91

Did you mean that you needed a dedicated api in NodeRequestHandler for this use-case?

@stanislavHamara
Copy link
Author

Yes, I was trying to get this done without forking. I am not sure where in the API would this fit but as mentioned above, the current message in most of the use cases is not very suitable as the user will have other tabs open. Being able to customize the message would be greatly beneficial from a UX point of view.

@tikurahul
Copy link
Collaborator

I can see that this will be useful to more developers. I will add this API to NodeBasedHandler.

@francisco-maciel
Copy link

This is quite important, especially if we take into account the need for internationalization.
@tikurahul do you still have plans to implement this? Or should I make a pull request?

@jakefeasel
Copy link
Contributor

The message shown is just part of the sample app, right? Fundamental use of the library (which admittedly isn't documented very clearly) doesn't show any messages to the user.

For a sample of using AppAuth without any sample-code intermingled within it, take a look at what I've done at https://github.com/ForgeRock/appAuthHelper

@tikurahul
Copy link
Collaborator

My original intention was that forks would allow for changing things like UI.
The library was mostly doing enough plumbing to get out of your way - and subsequent rebases would be cheap.

@shadow-light
Copy link

The message shown is just part of the sample app, right? Fundamental use of the library (which admittedly isn't documented very clearly) doesn't show any messages to the user.

For a sample of using AppAuth without any sample-code intermingled within it, take a look at what I've done at https://github.com/ForgeRock/appAuthHelper

No, it's not part of the sample app, it's part of node_request_handler.ts. It would be pretty trivial to allow a string to be passed to the constructor of NodeBasedHandler. It is not simple to extend that class just to change the response string unfortunately.

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

5 participants