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

Check OAuth only if `Authorization` header is OK #4123

Merged
merged 3 commits into from Nov 5, 2019

Conversation

@uxmaster
Copy link
Contributor

uxmaster commented Oct 31, 2019

So only if there is an Authorization header, and its value does not start with Basic.

This fixes the following errors:

@mic4ael
mic4ael approved these changes Nov 5, 2019
@mic4ael mic4ael force-pushed the uxmaster:uxmaster-patch-1 branch 2 times, most recently from e642c0d to f3d14ff Nov 5, 2019
@mic4ael mic4ael changed the base branch from master to 2.2-maintenance Nov 5, 2019
@mic4ael mic4ael force-pushed the uxmaster:uxmaster-patch-1 branch from f3d14ff to 68e06bc Nov 5, 2019
@mic4ael mic4ael force-pushed the uxmaster:uxmaster-patch-1 branch from 68e06bc to eb3c174 Nov 5, 2019
@ThiefMaster ThiefMaster merged commit b82bd70 into indico:2.2-maintenance Nov 5, 2019
@uxmaster

This comment has been minimized.

Copy link
Contributor Author

uxmaster commented Nov 7, 2019

Thanks a lot for merging. Could you merge it also into the indico:2.1-maintenance branch?

I'm asking because apparently, we have an old 2.1 branch at KEK and this is also the case at BNL (the original issue reporters).

@ThiefMaster

This comment has been minimized.

Copy link
Member

ThiefMaster commented Nov 7, 2019

I'm afraid not - this is not a critical fix that justifies backporting it to an otherwise unsupported version (and making a release).

  • If you don't use HTTP basic auth, you can just disable the error emails and not bother about the errors getting logged
  • If you do use HTTP basic auth, either configure your webserver to exempt the affected API routes from it or apply the patch directly to your code.

And of course, I strongly recommend you to update to 2.2 as soon as possible. We will include the fix in the next 2.2 patch release soon.

@uxmaster

This comment has been minimized.

Copy link
Contributor Author

uxmaster commented Dec 5, 2019

When do you plan to do the next 2.2 patch release?

@ThiefMaster

This comment has been minimized.

Copy link
Member

ThiefMaster commented Dec 5, 2019

I actually wanted to do it earlier this week but we were busy with some other things....

Tomorrow or monday should work though :)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
3 participants
You can’t perform that action at this time.