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

Return 401 in all cases of failure #3

Merged
merged 1 commit into from
Feb 13, 2023

Conversation

robsliwi
Copy link
Contributor

@robsliwi robsliwi commented Dec 5, 2018

In some cases buffalo would return a 500 instead
of a 401 when the authorization fails.
This commit should fix this behavior, see
discussion in Slack:
https://gophers.slack.com/archives/C3MSAFD40/p1543923920092000

@markbates
Copy link
Member

The tests are failing for this, can you please take a look? Thanks.

basicauth.go Outdated Show resolved Hide resolved
basicauth_test.go Outdated Show resolved Hide resolved
@robsliwi robsliwi force-pushed the fix/401_unauthorized branch from f390cb2 to 75d6a39 Compare April 3, 2019 15:12
@robsliwi
Copy link
Contributor Author

robsliwi commented Apr 3, 2019

Thanks for the contributing link, should have checked this before making the PR 😇
Updated and rebased it, you can have a second look if you want.

@sio4 sio4 self-assigned this Jul 23, 2022
@github-actions
Copy link

This PR is stale because it has been open 45 days with no activity. Remove stale label or comment. Otherwise, this will be closed in 7 days.

@github-actions github-actions bot added the stale label Sep 27, 2022
@sio4 sio4 added s: triage Some tests need to be run to confirm the issue and removed stale labels Sep 27, 2022
@github-actions
Copy link

This PR is stale because it has been open 45 days with no activity. Remove stale label or comment. Otherwise, this will be closed in 7 days.

@github-actions github-actions bot added the stale label Nov 12, 2022
@sio4 sio4 removed the stale label Nov 12, 2022
Copy link
Member

@sio4 sio4 left a comment

Choose a reason for hiding this comment

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

Thank you for your contribution, and sorry for the late review.

Could you please check the comments and fix them? I will proceed with this PR after that.
If you have no interest in this module anymore, not sure since it is already more than four years ago, just ignore this message. I will work on it soon.

basicauth.go Outdated Show resolved Hide resolved
go.mod Outdated Show resolved Hide resolved
@robsliwi robsliwi force-pushed the fix/401_unauthorized branch from fad503a to 0b7b952 Compare February 12, 2023 19:19
In some cases buffalo would return a 500 instead
of a 401 when the authorization fails.
This commit should fix this behavior, see
discussion in Slack:
https://gophers.slack.com/archives/C3MSAFD40/p1543923920092000
@robsliwi robsliwi force-pushed the fix/401_unauthorized branch from 0b7b952 to 10445ab Compare February 12, 2023 19:23
@robsliwi
Copy link
Contributor Author

Thanks for picking this up!
Was quite a joy to clone the repo again, set up some Golang-dev-env and get this fixed.

Rebased, pushed, realized two commits, squashed them and all of the requested changes should be included by now 🤞

@robsliwi robsliwi requested a review from sio4 February 12, 2023 19:25
@sio4 sio4 added this to the v1.0.10 milestone Feb 13, 2023
@sio4 sio4 dismissed markbates’s stale review February 13, 2023 13:47

all the reviews were addressed and Mark is not actively work on here nowadays

@sio4 sio4 merged commit a78e58d into gobuffalo:main Feb 13, 2023
@sio4
Copy link
Member

sio4 commented Feb 13, 2023

Thanks!

@sio4
Copy link
Member

sio4 commented Feb 13, 2023

Just for your information, I made some more changes to the topic and filed PR #7. Just in case you are interested.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
s: triage Some tests need to be run to confirm the issue
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants