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

Refactor authentication #305

Merged
merged 4 commits into from
Dec 15, 2023

Conversation

Edznux
Copy link
Contributor

@Edznux Edznux commented Jun 25, 2023

Follow up of this #286, happy to discuss.

This PR adds:

  • exports errors (so we can use errors.Is() in other package)
  • Add tests
  • Extracts SQL Queries

Notes:
The line token := c.Request.Header.Get("apitoken") is IMO missleading: the actual header is "Apitoken" because it's canonical-ified, and thus is equal to token := c.Request.Header.Get("Apitoken") (note the capital A)
What do you think about updating it to the last one to match what's the user is supposed to provide? (Apitoken and not apitoken)

@its-a-feature
Copy link
Owner

its-a-feature commented Jun 30, 2023

I'm a little confused about the header update you're mentioning. From what I could see, HTTP header values are case insensitive, so it shouldn't matter? Mythic's Scripting PyPi package for example (https://github.com/MythicMeta/Mythic_Scripting/blob/master/mythic/mythic_utilities.py#L52) just sends the header along as apitoken. Are you suggesting updating it to Apitoken just to match the normal canonical style of capitalization?

@its-a-feature its-a-feature merged commit 12759e8 into its-a-feature:master Dec 15, 2023
@Edznux Edznux deleted the refactor-authentication branch January 8, 2024 19:18
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.

2 participants