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.

Sign up for GitHub

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

Jump to bottom

Rate-limiting settings being loaded from node-config instead of application #382

Open
1 of 2 tasks
christian-hawk opened this issue Nov 12, 2021 · 2 comments · May be fixed by #414
Open
1 of 2 tasks

Rate-limiting settings being loaded from node-config instead of application #382

christian-hawk opened this issue Nov 12, 2021 · 2 comments · May be fixed by #414
Labels
bug high-priority

Comments

@christian-hawk
Copy link
Contributor

christian-hawk commented Nov 12, 2021 •
edited
Loading

We use node-config module to handle settings that changes according to NODE_ENV (i.e. TEST, PRODUCTION, DEV).

gluu-passport/config/default.js

Line 7 in fc37c63

rateLimitWindowMs: 24 * 60 * 60 * 1000,

We ended delegating some application configuration to node-config , as would not depend on changing other modules given the urgency in the past. This is not only a design issue, but it reflects in production bug, as Cloud Native users won't be able to change the values in production.js file.

We need to move this setting to where other applications settings are: backend persistence.

This should be:
This was referenced Nov 12, 2021
Closed
Closed
@christian-hawk christian-hawk mentioned this issue Nov 15, 2021
Merged
@kdhttps
Copy link
Contributor

kdhttps commented Dec 14, 2021

@christian-hawk we have below config in production.js

  passportFile: '/etc/gluu/conf/passport-config.json',
  saltFile: '/etc/gluu/conf/salt',
  timerInterval: 60000,
  rateLimitWindowMs: 24 * 60 * 60 * 1000, // 24 hrs in milliseconds
  rateLimitMaxRequestAllow: 1000,
  cookieSameSite: 'none',
  cookieSecure: true,
  HTTP_PROXY: process.env.HTTP_PROXY,
  HTTPS_PROXY: process.env.HTTPS_PROXY,
  NO_PROXY: process.env.NO_PROXY

so which one do we want to config through oxtrust(backend persistence)? I think except passportFile and saltFile, we can take all. Let me know your thought

@kdhttps kdhttps linked a pull request Jan 20, 2022 that will close this issue
Open
3 tasks
@christian-hawk
Copy link
Contributor Author

@christian-hawk we have below config in production.js

  passportFile: '/etc/gluu/conf/passport-config.json',
  saltFile: '/etc/gluu/conf/salt',
  timerInterval: 60000,
  rateLimitWindowMs: 24 * 60 * 60 * 1000, // 24 hrs in milliseconds
  rateLimitMaxRequestAllow: 1000,
  cookieSameSite: 'none',
  cookieSecure: true,
  HTTP_PROXY: process.env.HTTP_PROXY,
  HTTPS_PROXY: process.env.HTTPS_PROXY,
  NO_PROXY: process.env.NO_PROXY

so which one do we want to config through oxtrust(backend persistence)? I think except passportFile and saltFile, we can take all. Let me know your thought

agree.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
bug high-priority
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

Centralize one source for configuration GluuFederation/gluu-passport
2 participants
@kdhttps @christian-hawk