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

STCLI-246 Add a proxy server to overcome issues with cookies SameSite policy. #350

Merged
merged 2 commits into from
Jun 18, 2024

Conversation

BogdanDenis
Copy link
Contributor

Description

After updates that involve using Cookies for authentication, developers could face issues that cookies wouldn’t be included in requests

Browsers refused to set cookies due to “SameSite=”Lax” attribute:
image
Since this is a browser limitation we can overcome this by setting up a local proxy server which would redirect requests to Okapi and back.

This server is included as part of stripes-cli, so that developers don't have to install any third-party software.
Added two new options to serve command:

  • --startProxy
  • --proxyPort

Screenshots

ConEmu64_siErZJYdi2.mp4

Issues

STCLI-246

@BogdanDenis BogdanDenis requested review from zburke, Dmytro-Melnyshyn and a team June 18, 2024 10:31
Copy link

github-actions bot commented Jun 18, 2024

Jest Unit Test Statistics

0 tests  ±0   0 ✔️ ±0   0s ⏱️ ±0s
0 suites ±0   0 💤 ±0 
0 files   ±0   0 ±0 

Results for commit a79d797. ± Comparison against base commit adafbcf.

♻️ This comment has been updated with latest results.

@@ -5,6 +5,17 @@ module.exports.serverOptions = {
default: 3000,
group: 'Server Options:',
},
startProxy: {

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should we have default value false as described in commands.md file?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That's a good idea, @artem-blazhko ; all the other options here include a default property. Sure, undefined is already falsey so it won't change anything, but it's good to be explicit.

@artem-blazhko artem-blazhko requested a review from a team June 18, 2024 10:39
Copy link
Member

@zburke zburke left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

OMG, thank you ❤️ It's a small bit of work, but it's just the right work! I'm really pleased to see this built into stripes-cli, making it dead simple for anybody to use.

doc/commands.md Outdated
@@ -1146,6 +1148,10 @@ Serve a build previously created with "stripes build":
```
$ stripes serve --existing-build output
```
Serve a platform with a local proxy server that points to remove okapi server:
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Petty, typo: s/remove/remote/

@@ -5,6 +5,17 @@ module.exports.serverOptions = {
default: 3000,
group: 'Server Options:',
},
startProxy: {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That's a good idea, @artem-blazhko ; all the other options here include a default property. Sure, undefined is already falsey so it won't change anything, but it's good to be explicit.

@@ -11,6 +11,7 @@ const defaultConfig = {
showPerms: false,
hasAllPerms: false,
languages: ['en'],
useSecureTokens: false,
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why default to false? This will hard-coded to true in Sunflower, if not Ramsons.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I set false because it wasn't present in here before. But since we have it as true in platform config then it doesn't hurt to set to true here as well

Copy link

Quality Gate Failed Quality Gate failed

Failed conditions
1 Security Hotspot
0.0% Coverage on New Code (required ≥ 80%)

See analysis details on SonarCloud

@BogdanDenis BogdanDenis merged commit 5b4700f into master Jun 18, 2024
4 of 5 checks passed
@BogdanDenis BogdanDenis deleted the STCLI-246 branch June 18, 2024 13:24
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

Successfully merging this pull request may close these issues.

4 participants