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

feat: cookie parameters in proxy configuration #363

Closed
wants to merge 1 commit into from

Conversation

peppelinux
Copy link
Member

@peppelinux peppelinux commented Mar 30, 2021

This PR fixes #344 introducing the following paramenters to be optionally handled in the proxy global configuration.

Secure
Now can be disabled via COOKIE_SECURE: no in proxy_conf.yaml.
Default: True
that's only for dev purpose!

HttpOnly
To avoid cross-site scripting (XSS) attacks, cookies set with the HttpOnly directive are inaccessible to the JavaScript Document.cookie API. For example, session cookies don't need to be accessed by JavaScript and should therefore be set with the HttpOnly flag.
Default: True
parameter name eg: COOKIE_HTTPONLY: no

Domain, COOKIE_DOMAIN
Max-Age, COOKIE_MAX_AGE

it come also with some minor code linting and f-strings, few things.

All Submissions:

  • Have you checked to ensure there aren't other open Pull Requests for the same update/change?
  • Have you added an explanation of what problem you are trying to solve with this PR?
  • Have you added information on what your changes do and why you chose this as your solution?
  • Have you written new tests for your changes?
  • Does your submission pass tests?
  • This project follows PEP8 style guide. Have you run your code against the 'flake8' linter?

Secure
Now can be disabled via `COOKIE_SECURE: no` in proxy_conf.yaml. Default: True

HttpOnly
To avoid cross-site scripting (XSS) attacks, cookies set with the HttpOnly directive are inaccessible to the JavaScript Document.cookie (en-US) API; they are only sent to the server. For example, session cookies do not need to be accessed by JavaScript and should therefore be set with the HttpOnly flag.
Default: True
proxy_conf.yaml parameter name eg: `COOKIE_HTTPONLY: no`

minor code linting
@peppelinux
Copy link
Member Author

It seems that here we have a collision

cookie = SimpleCookie()

anyway, with my PR I tested in the user-agent debugger and it seems that SATOSA cookie respects the global configuration parameter, instead of the frontend ones.

I think that we should have to decide if keep cookie paramenters in the plugins or move them in the global configuration and refactor/cleanup/reduce all those hardcodings

@c00kiemon5ter
Copy link
Member

@peppelinux we will be merging this PR.

The SimpleCookie created by the frontend is a separate one, it has a different purpose and should not conflict the cookie that the proxy is using to store state.

@c00kiemon5ter c00kiemon5ter self-assigned this Apr 20, 2021
@c00kiemon5ter c00kiemon5ter added the next-release should become part of the next release label Nov 16, 2021
@c00kiemon5ter
Copy link
Member

closed by 1206ea5

@peppelinux
Copy link
Member Author

peppelinux commented Jun 11, 2023

This PR should have been taken

Since the code committed Is equal or similar

There are some cases where a PR Is not perfect but it's 80% good

In these cases the mantainers can add their commits Upon the PR made by the community

This preserves the contribution in the history of the commits, giving a gratification to contributions

If this doesn't happen, what would be the value to contribute in a project?

This Is not project management or product design, this Is social Exchange and I think that our community needs this awareness, then we should be able to apply these mechanisms to gather contributions and value in the projects

@c00kiemon5ter
Copy link
Member

@peppelinux you are right and I will try to do that in the future ❤️

@peppelinux
Copy link
Member Author

Even if the priority Is the release (output, the "what") the process to achieve this is important (the how)

I'm Happy of this brand new release, at the same time we may had a pull request to understand the "what" and the "how"

We should also take in account that a release that introduces these features needs testing, because i dont know which Will be the impact of this release on the systems in production and there might be the possibility to test these in a PR/branch, before a release

This Is just for sharing, I look forward with you

Ad maiora

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
next-release should become part of the next release
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[Cookie parameters] Better to handle them in the general proxy_conf.yaml
2 participants