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

Do you care about managing vulnerabilities? #55

Open
adriatic opened this issue Oct 30, 2018 · 18 comments
Open

Do you care about managing vulnerabilities? #55

adriatic opened this issue Oct 30, 2018 · 18 comments

Comments

@adriatic
Copy link

adriatic commented Oct 30, 2018

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/[email protected] 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: [email protected] (node_modules\fsevents):                                          
npm WARN notsup SKIPPING OPTIONAL DEPENDENCY: Unsupported platform for [email protected]: 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 [email protected] to resolve 4 vulnerabilities
  2. Run npm install @okta/[email protected] to resolve 3 vulnerabilities
  3. Run npm install @okta/[email protected] 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
Copy link
Contributor

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
Copy link
Author

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
Copy link
Author

adriatic commented Aug 27, 2020

@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: [email protected] (node_modules\fsevents):
npm WARN notsup SKIPPING OPTIONAL DEPENDENCY: Unsupported platform for [email protected]: 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
Copy link
Contributor

mraible commented Aug 28, 2020

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

@adriatic
Copy link
Author

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
Copy link
Contributor

mraible commented Aug 28, 2020

@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
Copy link
Contributor

mraible commented Aug 28, 2020

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. oktadev/okta-blog#386

@adriatic
Copy link
Author

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
Copy link
Author

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
Copy link

shuowu commented Aug 31, 2020

@adriatic Per #55 (comment), does add your redirectUri http://localhost:3000/callback to your okta client app help?

@adriatic
Copy link
Author

adriatic commented Aug 31, 2020

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

@adriatic adriatic reopened this Aug 31, 2020
@shuowu
Copy link

shuowu commented Aug 31, 2020

@adriatic From the url here #55 (comment) , looks like /users is missing by some reason.

@shuowu
Copy link

shuowu commented Sep 1, 2020

@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
Copy link
Author

adriatic commented Sep 1, 2020

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
Copy link
Contributor

mraible commented Sep 1, 2020

@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
Copy link
Author

adriatic commented Sep 1, 2020

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

@adriatic
Copy link
Author

adriatic commented Sep 4, 2020

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'
  },
 }
});

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

No branches or pull requests

4 participants