Skip to content

Conversation

@gertminov
Copy link

resolves #78

Checklist

Copy link
Member

@mcollina mcollina left a comment

Choose a reason for hiding this comment

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

lgtm

@mcollina mcollina requested a review from jsumners March 29, 2022 15:58
Copy link
Member

@jsumners jsumners left a comment

Choose a reason for hiding this comment

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

I am unclear about the response example, and I think the plugin is pretty clear that it implements basic authentication (i.e. https://www.rfc-editor.org/rfc/rfc7617.html). So it seems redundant to me to add an example, but I suppose it doesn't hurt anything either. I would just remove the "send credentials back the client" portion as it isn't spec compliant and is a glaring security violation.

README.md Outdated
### How to add credentials to a request
The basic-auth plugin retrieves the credentials from the [Authorization](https://en.wikipedia.org/wiki/Basic_access_authentication) header.

A simple example to add `username` and `password` to the reply header:
Copy link
Member

Choose a reason for hiding this comment

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

Why would we give an example showing sending the credentials in a response header? https://www.rfc-editor.org/rfc/rfc7617.html does not call for this, and it seems like a bad practice to recommend.

README.md Outdated
fetch('URL_GOES_HERE', {
method: 'post',
headers: new Headers({
'Authorization': 'Basic '+btoa('username:password'),
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
'Authorization': 'Basic '+btoa('username:password'),
'Authorization': 'Basic ' + btoa('username:password'),

- removed node example
- changed misspelling
- added spacing
- changed 'URL_GOES_HERE' to 'https://www.example.org/'
@stale
Copy link

stale bot commented Apr 16, 2022

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.

@stale stale bot added the stale label Apr 16, 2022
@stale stale bot closed this Apr 27, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Add example of how to add credentials to request to README

5 participants