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

Error handling for non-https #15

Open
alexdahl-okta opened this issue Mar 28, 2017 · 2 comments
Open

Error handling for non-https #15

alexdahl-okta opened this issue Mar 28, 2017 · 2 comments
Labels

Comments

@alexdahl-okta
Copy link

tl;dr In the config for oktaUrl, I accidentally used http instead of https. The resulting error is obscure, and I had a hard time troubleshooting.

Steps to reproduce

  1. Follow the OIDC setup to create a new organization and app
  2. Follow the readme quick start, but use"oktaUrl": "http://{{yourOktaOrg}}.oktapreview.com" instead of https.
  3. Run npm start and open localhost:3000.
  4. Go to "Log in by redirecting to Okta" and click "Sign in".
  5. Submit the Okta sign in form with your credentials

Result

"id_token could not be decoded from the response":
result

I tried to debug by printing the token. I searched my local source for the error message, found route-handlers.js:174, and added + json to the status message. This revealed a redirect message instead of a token:
screen shot 2017-03-27 at 12 39 12 pm

Following the redirect link revealed a server error response:
screen shot 2017-03-27 at 12 39 15 pm

I did not mentally connect this back to my config URL. It wasn't until I was comparing my config file with someone else's that I noticed the http/https difference.

Thanks to @nbarbettini for helping me debug!

Potential improvements

  • Catch non-token responses and throw a more descriptive error. It's true that the token could not be decoded, but that's because it wasn't actually a JWT to start with.
  • Use an https oktaUrl in the default .samples.config.json, to make safe copy-pastes that don't include the protocol. This might align with issue (Confusing) README for mock vs. real org #14.
  • Improve E0000022's documentation (or summary) to clearly state that https is required, not merely that http is not supported.
@nbarbettini
Copy link
Contributor

Thanks for the great issue reporting @alexdahl-okta 😄

Another potential fix is inspecting the config at startup, and throwing a warning or error if the oktaUrl is http but NOT localhost.

@nbarbettini nbarbettini changed the title Error handling for non-token responses Error handling for non-https Jan 18, 2018
@nbarbettini
Copy link
Contributor

Since we rebuilt all these samples from scratch, I tested this again. If I use http instead of https in webServer.oidc.issuer in .samples.config.json, the app refuses to start with a cryptic error message:

C:\Users\Nate\Documents\code\samples-nodejs-express-4 [master ≡ +0 ~2 -0 !]> npm run okta-hosted-login-server
(node:18300) UnhandledPromiseRejectionWarning: Unhandled promise rejection (rejection id: 1): SyntaxError: Unexpected token < in JSON at position 0
(node:18300) [DEP0018] DeprecationWarning: Unhandled promise rejections are deprecated. In the future, promise rejections that are not handled will terminate the Node.js process with a non-zero exit code.

Since the config file now contains https by default, I don't think many people will run into this. Still, it'd be nice to output a warning if the issuer does not begin with https. And, if the discovery document can't be parsed, throw a meaningful error message.

cc @robertdamphousse-okta

@robertjd robertjd added the to do label Apr 19, 2018
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

3 participants