-
Notifications
You must be signed in to change notification settings - Fork 2.1k
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
OAuth2 App enabled with CORS fix #28864
Conversation
👍 |
@noveens also we made CORS allowed by default for all OCS endpoints. |
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.
👍
@noveens please add this one to your backport PR |
Jenkins slapped |
can you restart travis? |
For Travis, you can click the "Details" link, select the job(s) that had a problem and press "restart". |
Are we fully droping the CORS annotation? Please verify that this is still working - and dev docs need to be updated |
@noveens please verify. I suspect that even if the annotation is set, it will not break as CORS is enabled by default. Better check though to avoid surprises. |
We are not dropping the Like all the methods with |
This thread has been automatically locked since there has not been any recent activity after it was closed. Please open a new issue for related bugs. |
Description
I removed the beforeController logic here due to the change of handling CORS since PR #28457
According to previous implementation, CORS was only allowed with methods that had
@PublicPage
notation for preventing CSRF attacks.But in the latest PR by me, the current implementations is as follows:
This implementation removes the need for
@PublicPage
.Related Issue
#28860
How Has This Been Tested?
Tested by @SamuAlfageme
More testing required since I can't reproduce the issue.
Types of changes
Checklist:
@PVince81 @jesmrec @SamuAlfageme @Peter-Prochaska