-
Notifications
You must be signed in to change notification settings - Fork 3
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
Nexus integration - pypi, npm #3188
Conversation
proxy_set_header Host $host; | ||
proxy_set_header X-Real-IP $remote_addr; | ||
proxy_set_header X-Forwarded-For $proxy_add_x_forwarded_for; | ||
proxy_set_header X-Forwarded-Proto https; | ||
#proxy_set_header X-Forwarded-Proto https; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
#proxy_set_header X-Forwarded-Proto https; | |
proxy_set_header X-Forwarded-Proto https; |
…into bug/3141-e2e-failing
Scenario: A new user is approved and can see the app homepage | ||
Scenario: A new user requests access | ||
Given The admin logs in | ||
And '[email protected]' is in begin state | ||
When '[email protected]' visits the home page | ||
And '[email protected]' logs in | ||
Then '[email protected]' requests access | ||
And The admin sets the approval status of '[email protected]' to 'Approved' | ||
When '[email protected]' requests access | ||
Then '[email protected]' sees the request still submitted | ||
Scenario: A new user is approved and can see the app homepage | ||
Given The admin logs in | ||
When '[email protected]' visits the home page | ||
Then The admin sets the approval status of '[email protected]' to 'Approved' | ||
And '[email protected]' logs in |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
By splitting this into two scenarios, we are side-stepping some issue with csrf and/or csrfMiddlewaretoken that cypress is not handling for the two users. This was able to get a local cypress pointed at develop to pass tests successfully but leaves technical debt on why the tokenization steps are failing in the back and forth of requests and responses. We were also seeing 302 and then 200 even on failed POSTS/PATCHES indicating there's a flaw in our user code on the django api view.
form: true, | ||
headers: { | ||
Referer: `${Cypress.env('adminUrl')}`, | ||
Referer: `${Cypress.env('adminUrl')}${path}`, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is likely not needed by more closely mimics what I see via dev tools in Firefox.
@@ -5,6 +5,7 @@ location ~ ^/(v1|admin|static/admin|swagger|redocs) { | |||
set $backend http://{{env "BACKEND_HOST"}}.apps.internal:8080$request_uri; | |||
proxy_pass $backend; | |||
proxy_pass_header x-csrftoken; | |||
proxy_set_header Referer {{env "ALLOWED_ORIGIN" }}; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Probably un-needed.
Summary of Changes
Pull request closes #3171
How to Test
TBD
Deliverables
More details on how deliverables herein are assessed included here.
Deliverable 1: Accepted Features
Checklist of ACs:
lfrohlich
and/oradpennington
confirmed that ACs are met.Deliverable 2: Tested Code
CodeCov Report
comment in PR)CodeCov Report
comment in PR)Deliverable 3: Properly Styled Code
Deliverable 4: Accessible
iamjolly
andttran-hub
using Accessibility Insights reveal any errors introduced in this PR?Deliverable 5: Deployed
Deliverable 6: Documented
Deliverable 7: Secure
Deliverable 8: User Research
Research product(s) clearly articulate(s):