You are viewing a plain text version of this content. The canonical link for it is here.
Posted to pr@flagon.apache.org by "Jyyjy (via GitHub)" <gi...@apache.org> on 2024/02/06 16:48:50 UTC
[PR] Ext auth (flagon-useralejs)
Jyyjy opened a new pull request, #408:
URL: https://github.com/apache/flagon-useralejs/pull/408
- Adds a password setting to be used with userId for basic http auth.
- Exposes it to the browser plugin.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: pr-unsubscribe@flagon.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: pr-unsubscribe@flagon.apache.org
For additional commands, e-mail: pr-help@flagon.apache.org
Re: [PR] Ext auth (flagon-useralejs)
Posted by "Jyyjy (via GitHub)" <gi...@apache.org>.
Jyyjy merged PR #408:
URL: https://github.com/apache/flagon-useralejs/pull/408
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: pr-unsubscribe@flagon.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: pr-unsubscribe@flagon.apache.org
For additional commands, e-mail: pr-help@flagon.apache.org
Re: [PR] Ext auth (flagon-useralejs)
Posted by "EandrewJones (via GitHub)" <gi...@apache.org>.
EandrewJones commented on code in PR #408:
URL: https://github.com/apache/flagon-useralejs/pull/408#discussion_r1480269429
##########
src/sendLogs.js:
##########
@@ -89,6 +89,12 @@ export function sendLogs(logs, config, retries) {
req.open("POST", config.url);
// Update headers
+ if( config.userId && config.password) {
Review Comment:
There's a bit of a code smell here. Silent, unexpected behavior may emerge.
If I set a password as well as an authHeader callback, the latter will overwrite the basic authorization with whatever my function returns. We now expose multiple ways to do the same thing: customer headers callback, authHeaders callback, and this password setting.
I don't expect users to use both password _and_ the custom callback, but exposing too many ways to do things can be confusing. _We really should force all of this into the callback strategy pattern._ The problem is the callback doesn't accept arguments and we have no way to persist the password unless we store it on the config. However, as discussed, storing the password is also poor practice. That said, so is basic auth...
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@flagon.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@flagon.apache.org
For additional commands, e-mail: notifications-help@flagon.apache.org
Re: [PR] Ext auth (flagon-useralejs)
Posted by "Jyyjy (via GitHub)" <gi...@apache.org>.
Jyyjy commented on code in PR #408:
URL: https://github.com/apache/flagon-useralejs/pull/408#discussion_r1480307816
##########
src/sendLogs.js:
##########
@@ -89,6 +89,12 @@ export function sendLogs(logs, config, retries) {
req.open("POST", config.url);
// Update headers
+ if( config.userId && config.password) {
Review Comment:
Completely agree, I was thinking about either
1. Removing static auth header option. Basic auth can be used with username/password, I think all other [auth schemes](https://www.iana.org/assignments/http-authschemes/http-authschemes.xhtml) are better served by a callback.
2. Not including a password setting in the main package, only in the browser extension. But I am trying to keep settings consistent, so it's clear what does what.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@flagon.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@flagon.apache.org
For additional commands, e-mail: notifications-help@flagon.apache.org