search

okta / samples-nodejs-express-4

Express 4 samples. Will publish an artifact that can be consumed by end-to-end sample repos
Other
120 stars 118 forks source link

Do you care about managing vulnerabilities? #55

Open adriatic opened 5 years ago

adriatic commented 5 years ago

I encountered this same issue at a different okta sample and described the problem as well as presented the solution. As nobody responded, I could think that keeping the samples current exceeds the okta team's ability to do - let me please know if that is the case; I would then stop writing such observations πŸ˜„

Running npm install for the Express & Okta-Hosted Login Page Example results with:

Ξ» npm install                                                                                                                    

> @okta/samples-nodejs-express-4@3.0.0 postinstall c:\work\learning\okta\samples-nodejs-express-4                                
> node post-install.js                                                                                                           

Creating default configuration file                                                                                              

Sample project is ready to go!  Please add your configuration to c:\work\learning\okta\samples-nodejs-express-4\.samples.config.j
son, see the README for instructions.                                                                                            

npm WARN optional SKIPPING OPTIONAL DEPENDENCY: fsevents@1.1.3 (node_modules\fsevents):                                          
npm WARN notsup SKIPPING OPTIONAL DEPENDENCY: Unsupported platform for fsevents@1.1.3: wanted {"os":"darwin","arch":"any"} (curre
nt: {"os":"win32","arch":"x64"})                                                                                                 

added 380 packages from 351 contributors and audited 1332 packages in 14.251s                                                    
found 25 vulnerabilities (4 low, 16 moderate, 5 high)                                                                            
  run `npm audit fix` to fix them, or `npm audit` for details

Subsequent execution of the npm audit results with the following suggestions:

  1. Run npm install --save-dev protractor@5.4.1 to resolve 4 vulnerabilities
  2. Run npm install @okta/oidc-middleware@1.0.2 to resolve 3 vulnerabilities
  3. Run npm install @okta/jwt-verifier@0.0.14 to resolve 3 vulnerabilities
  4. Run npm update fsevents --depth 3 to resolve 13 vulnerabilities
  5. Run npm update fill-range --depth 7 to resolve 1 vulnerability

I did try to run these suggested commands, only to find that the total number of vulnerabilities dropped from 25 to 16, meaning that there is more work to be done, because of various inter-dependencies.

nbarbettini commented 5 years ago

Hey @adriatic, thanks for the report. We do care about managing vulnerabilities! We're working on these now and will update the samples soon.

adriatic commented 5 years ago

Great to see that you provided the only reasonable response to my question - and that you responded so quickly, @nbarbettini. As I am preparing for a large project in the healthcare domain for one of your important customers, your reaction gives me a great relief πŸ˜„ Thank you Nate.

adriatic commented 3 years ago

@nbarbettini almost two years passed since your comment above - and running npm install on that sample code results with

added 488 packages from 421 contributors and audited 557 packages in 8.327s
found 64 vulnerabilities (62 low, 2 high)
  run `npm audit fix` to fix them, or `npm audit` for details'

After running this command, the situation seems a lot better:

npm WARN optional SKIPPING OPTIONAL DEPENDENCY: fsevents@1.2.9 (node_modules\fsevents):
npm WARN notsup SKIPPING OPTIONAL DEPENDENCY: Unsupported platform for fsevents@1.2.9: wanted {"os":"darwin","arch":"any"} (current: {"os":"win32","arch":"x64"})

updated 13 packages in 3.908s
fixed 58 of 64 vulnerabilities in 557 scanned packages
  5 vulnerabilities required manual review and could not be updated
  1 package update for 1 vulnerability involved breaking changes
  (use `npm audit fix --force` to install breaking changes; or refer to `npm audit` for steps to fix these manually)

After this correction, there are several other instructions on how to bring this app to level that has zero vulnerabilities - the problem is that I am not doing any help to run these mods on my own workstation.

I would gladly supply a PR - but it is highly likely that it would just sit there in the repo 😭 )

Please note that this code sample is the very first referenced in your guides

mraible commented 3 years ago

@adriatic If you provide a PR that passes all CI tests, I'll do my best to get it merged.

adriatic commented 3 years ago

Hello, Matt (@mraible) - you really made me happy with not just your response, but also with its timing. In addition to just update this sample, I would propose a more advanced approach, which should result in more benefits for Okta: Let's create a "movement" characterized by rallying Octa community members to ensure that all of the "official samples and blogs" are refreshed at all times.

Quite seriously - it would be Okta newbies that would benefit the most from this action, and these folks are one of the main keys for Okta's good future. Please note also that in order to do the complete refreshing, one would also need source text for the blogs that are tutorials for the code in GitHub

In order to verify me, please check my friend's Randall Degges opinion of me πŸ˜ƒ

mraible commented 3 years ago

@adriatic I agree with you. Your timing is impeccable too! I tried updating all our JS SDK samples last week and ran into some issues. The good news is most of these PRs are progressing.

I just created a new one for this repo.

I'm letting the CI process do most of the work for these and then checking in to try to fix the errors periodically. If folks want to create PRs to fix my PRs, that'd be great! πŸ˜ƒ

mraible commented 3 years ago

Please note also that in order to do the complete refreshing, one would also need source text for the blogs that are tutorials for the code in GitHub

You can find our blog repo at https://github.com/oktadeveloper/okta-blog.

We do our best to keep our blog posts working, but it can be difficult. I updated a post from March 2017 today. https://github.com/oktadeveloper/okta-blog/pull/386

adriatic commented 3 years ago

Hello, @mraible

Your timing is impeccable too!

This makes me happy - as my motives to do such a relatively pedestrian job of refreshing old code samples and related (markup) texts are driven by my many years' old affinity for Okta, (Les Hazelwood and Randall Degges from Stormpath for example). In addition, I am trying to help my friends at Strapi to understand the reasons to use an IAM PaaS instead of some home-created API.

My plan is to use several Okta Node samples upgraded to today's API - and then rework them to fit the full-stack Strapi tutorials. This additional work is also my contribution to Okta - this time in the form of Okta marketing efforts

The log of my activities leading to the creation of set of PRs is at https://github.com/adriatic-okta-prs/status/issues/1

adriatic commented 3 years ago

I also have a question: all of the code samples I tried to upgrade result with the same failure:

image

The related URL is https://dev-487304.okta.com/oauth2/default/v1/authorize?client_id=0oarifhve59q3PrM84x6&scope=openid%20profile&response_type=code&redirect_uri=http%3A%2F%2Flocalhost%3A3000%2Fcallback&state=Ur3vuJgDr_yZynSKvPuxJx7tDZQPpZHyIkOlfyaaJu0

This ought to be a consequence of some API changes that took place after the samples are initially created.

Can you illuminate please?

shuowu commented 3 years ago

@adriatic Per https://github.com/okta/samples-nodejs-express-4/issues/55#issuecomment-684054359, does add your redirectUri http://localhost:3000/callback to your okta client app help?

adriatic commented 3 years ago

I believe that it does here is the relevant section of the code

const { ExpressOIDC } = require('@okta/oidc-middleware');
const oidc = new ExpressOIDC({
 appBaseUrl: process.env.HOST_URL,
 issuer: `${process.env.OKTA_ORG_URL}/oauth2/default`,
 client_id: process.env.OKTA_CLIENT_ID,
 client_secret: process.env.OKTA_CLIENT_SECRET,
 redirect_uri: `${process.env.HOST_URL}/users/callback`,
 scope: 'openid profile',
 routes: {
  loginCallback: {
   path: 'users/callback'
  },
 }
});

as well as the configuration:

image

shuowu commented 3 years ago

@adriatic From the url here https://github.com/okta/samples-nodejs-express-4/issues/55#issuecomment-684054359 , looks like /users is missing by some reason.

adriatic commented 3 years ago

Yes, @shuowu, you are correct - the url in my post above is older than it should be - here it the current one: https://dev-487304.okta.com/oauth2/default/v1/authorize?client_id=0oarifhve59q3PrM84x6&scope=openid%20profile&response_type=code&redirect_uri=http%3A%2F%2Flocalhost%3A3000users%2Fcallback&state=GtMfeyvOhFfLOuy8Zld0OyWB2Fo1UXKQEaj1gbvrmhU

The sample I am discussing here is Build Simple Authentication in Express in 15 Minutes by Braden Kelley.

Github repo https://github.com/oktadeveloper/okta-node-express-15-minute-auth-example

shuowu commented 3 years ago

@adriatic The 400 page usually is caused by redirect_uri mismatch as what shows in the description page. One suggestion is to also check if the client id is correct, otherwise I am not aware of how the 400 error comes out.

adriatic commented 3 years ago

Hello, @shuowu the Uri mismatch cannot be the cause, nor could client ID (verified it many times over) I believe that there is a mismatch between today's Okta API and the applications I created using the older Okta blogs (written in 2018). By that, I mean that there are some code-breaking changes in the Okta API being used in the app code described in these blogs.

I was hoping that Matt (@mraible) would point out such differences. Since he seems to be busy elsewhere I will go debug my code and find out what is really happening there.

mraible commented 3 years ago

@adriatic I don't think there's breaking changes in our API. However, there are likely breaking changes in SDKs (especially between major versions). If you want to see how to quickly add login to an Express app, check out my post on the subject: Node.js Login with Express and OIDC.

adriatic commented 3 years ago

Thanks @mraible - I just decide to use this specific sample as my guidance, so think alike😏

adriatic commented 3 years ago

Hello, @mraible and @shuowu Apologies for being stupid and blind at the same time. All my 400 bad request reports are caused by the same mistake:

Missing the definition of the routes

 routes: {
  loginCallback: {
   path: 'users/callback'
  },
 }

in the ExpressOIDC constructor

const { ExpressOIDC } = require('@okta/oidc-middleware');
const oidc = new ExpressOIDC({
 appBaseUrl: process.env.HOST_URL,
 issuer: `${process.env.OKTA_ORG_URL}/oauth2/default`,
 client_id: process.env.OKTA_CLIENT_ID,
 client_secret: process.env.OKTA_CLIENT_SECRET,
 redirect_uri: `${process.env.HOST_URL}/users/callback`,
 scope: 'openid profile',
 routes: {
  loginCallback: {
   path: 'users/callback'
  },
 }
});