-
Notifications
You must be signed in to change notification settings - Fork 413
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
feat: authenticate app with daemon #1298
Comments
Currently getting:
daemon shows: |
@tzarebczan I got this yesterday as well. My daemon did not have an The relevant daemon side check is here https://github.com/lbryio/lbry/blob/master/lbrynet/daemon/auth/server.py#L398 We need to figure out if the daemon just needs to provide a default setting or if the app needs to change this setting at install time. |
@kauffj spoke to @IGassmann last week and he said it should be done from the app side. @IGassmann, is setting allowed_origin to * the correct way to implement this? This also happens in non-dev mode as well. |
These are the changes Jack made which should help us understand how the app should authenticate - it would need to create a handshake with the daemon: lbryio/lbry-sdk@743ae59#diff-2571ced04af2d940be952f29361a40a0R160 Notes:
|
@tzarebczan please get issues created app side. More importantly, the sprint and release process needs to better handle this and this should be discussed in an upcoming retrospective (@jackrobison @lyoshenka @eukreign). If a daemon upgrade requires all apps using it to change their behavior in how they call it, this ought to be extremely prominently mentioned in release notes, and we ought to be identifying these issues and creating tickets appropriately (or at least letting key team members know). |
@kauffj agreed but I think there is some confusion. This was the issue created as a result of communication between Jack and Igor - which was to enable the app to use http authentication (not currently doing so). It's not a mandatory setting, so all previous apps should work correctly without it. The confusion lies in some other changes that went along with this which is causing the app/daemon communication problem - I'm trying to debug the source of the issue and will work with Jack to fix it. |
Findings:
Postman jsonrpc:
Not sure if it's the app doing something wrong, or if lbry side needs to be fixed. |
@jackrobison / @lyoshenka will be looking into this from the daemon side to ensure it's implemented properly. |
This check was removed from the daemon for now. |
@tzarebczan and @jackrobison What is the status of this? |
Authentification available with daemon v0.20.0rc6.
The text was updated successfully, but these errors were encountered: