AIP-38 Fix safari login loop for non ssl

[pierrejeambrun]

Safari has a stricter policy when it comes to secure cookies. I will not allow localhost to use such cookies (https is always required), which is not the case for other browser.

This will at least fix the case for the development mode. i.e safari will work in --dev-mode in breeze or by specifying the env variable and running the api_server.

It will still not work in non dev-mode via breeze though.

There is no issue for real development mode as long as people are using https to connect to their instance.

[ashb]

Does this mean that deployments must be using https as well?

[pierrejeambrun]

If deployment do not use https safari will block such secure cookies and indeed this will not work.

Is that a popular use case ? Do we need to support safari for that ?

[pierrejeambrun]

Maybe only enable the secure attribute when there is api.ssl_cert in the conf ? Actually that's a better idea I think, let me update that

[ashb]

Maybe only enable the secure attribute when there is api.ssl_cert in the conf ? Actually that's a better idea I think, let me update that

That alone isn't enough -- it is much more common to use some kind of proxy in front to handle the TLS termination (ALB, Nginx, Kubernetes Ingress etc.) so we'll have to look at something else (at least as well)

Well, this "fails working" which is the right thing we want for now, so lets go ahead and merge this for now, but probably create an issue to come back and revisit this with reverse proxies in mind.

[ashb]

It's possible we can look at the request.secure or something. We have this in our config

    proxy_fix_x_proto:
      description: |
        Number of values to trust for ``X-Forwarded-Proto``.
        See `Werkzeug: X-Forwarded-For Proxy Fix
        <https://werkzeug.palletsprojects.com/en/2.3.x/middleware/proxy_fix/>`__ for more details.
      version_added: 1.10.7
      type: integer
      example: ~
      default: "1"

Which in turn is available as request.is_secure

[pierrejeambrun]

Issue here #47878, I marked it for 3.0 bugfix for now, we can take a look after the feature freeze