feat(auth): support custom credentials for authenticated actions#293
Conversation
Adds an optional pre-issued token to MinerAuth so callers can authenticate without relying on a default password. - MinerAuth gains `token: Option<SecretString>` (additive; `new(user, pass)` is unchanged, token defaults to None) plus a `with_token` builder. - VNish (no username, password-only, also accepts a bearer token): when a token is set, use it directly; otherwise unlock with the password as before. - Python `set_auth(username, password, token=None)` exposes it.
The BraiinsOS web API authenticates with a bearer token like VNish; when a token is supplied, use it directly instead of logging in with the password. Default credentials remain the fallback.
|
What is the reasoning behind this? There isn't a tool that I'm aware of that uses this, and the code is all open source, so you don't have to worry about it stealing your passwords or something. This just seems like a more complicated way to use a miner API... |
|
The reasoning is about how a system like Home Assistant should get access to a device. A token is the modern way to do that: I can grant HA scoped, revocable access without either handing it my own (plaintext) device password, or inventing a second password just for HA that I then have to remember — realistically only a password manager makes that workable. I'm genuinely glad VNish offers this, and I'd like to be able to use it. It's not about distrusting the library — it's that I'd rather not be pushed toward less security than the firmware already allows. And since this is purely additive (a handful of lines, no change to how anyone else uses the API, nothing removed or constrained), I'd be really happy to see it included: it just gives token-capable firmwares a first-class path while leaving password auth exactly as it is. |
I can see a situation where this is useful, but it just doesn't feel very extensible... I have some changes I want to propose to the shape of this which will change some other parts of the lib, but it should be possible. Side note, I am not a fan of this mainly because of the use cases at scale, and it just complicates the API because less than 1% of 1% of 1% of miners will use this, but if it has any use case I suppose asic-rs should try to support it. |
| /// Attach a pre-issued auth token (e.g. a bearer token). Backends that | ||
| /// support token auth use it instead of logging in with the password. | ||
| pub fn with_token(mut self, token: impl Into<String>) -> Self { | ||
| self.token = Some(SecretString::from(token.into())); | ||
| self | ||
| } |
There was a problem hiding this comment.
Use from_token() once swapped to an enum.
| let token = match &self.auth.token { | ||
| Some(token) => token.expose_secret().to_string(), | ||
| None => { | ||
| self.authenticate(self.auth.password.expose_secret()) | ||
| .await? | ||
| } | ||
| }; |
There was a problem hiding this comment.
When converted to an enum, you can match on self.auth to use the token directly if needed.
Per @b-rowan's review on 256foundation#293: convert MinerAuth from a struct with an optional token into an enum so backends pattern-match on the auth kind: enum MinerAuth { UserAndPass(UserAndPassAuth), TokenAuth(SecretString) } - MinerAuth::new() -> UserAndPass; MinerAuth::from_token() -> TokenAuth (replaces with_token) - accessor helpers username()/password()/token() for the user/pass call sites - VNish + BraiinsOS HTTP ensure_authenticated() now match on &self.auth to use the token directly or log in with username/password - python set_auth picks the variant from the optional token
|
Done — converted enum MinerAuth { UserAndPass(UserAndPassAuth), TokenAuth(SecretString) }
CI is green. You mentioned broader shape-changes that touch other parts of the lib — happy to go that way too. If you'd rather drive that shape, just outline it and I'll rework on top; this is a working first cut of the enum either way. |
| #[pyo3(signature = (username, password, token=None))] | ||
| pub fn set_auth( | ||
| &mut self, | ||
| username: String, | ||
| password: String, | ||
| token: Option<String>, | ||
| ) -> PyResult<()> { | ||
| let auth = match token { | ||
| Some(token) => MinerAuth::from_token(token), | ||
| None => MinerAuth::new(username, password), | ||
| }; | ||
| Arc::get_mut(&mut self.inner) | ||
| .ok_or_else(|| PyRuntimeError::new_err("cannot set auth while miner is in use"))? | ||
| .get_mut() | ||
| .set_auth(MinerAuth::new(username, password)); | ||
| .set_auth(auth); | ||
| Ok(()) | ||
| } |
There was a problem hiding this comment.
I could see this being a bit weird, could you add set_token which sets auth with a token, and leave this as-is with only username and password?
|
Not much left now, just want to separate out the python calls and then it will be good to merge. |
Per review: keep `set_auth(username, password)` user/pass-only and add a separate `set_token(token)` for pre-issued bearer tokens (e.g. VNish), instead of an optional `token` param. Shared `apply_auth` helper.
pos-ei-don
force-pushed
the
feat-auth-token-upstream
branch
from
3f6c4d0 to
d8339a4
Compare
|
Done in CI green (Cargo + Python). Should be good to merge now 👍 |
2 participants
A number of actions need authentication (config/metadata reads, presets, throttle, power target, …). This lets a consumer supply their own credentials when they have them — for VNish and BraiinsOS (BOS) — but it changes nothing unless they opt in.
If you supply no credentials, behaviour is identical to today: the backend logs in with its built-in default credentials, exactly as before. No default is removed; the connection probe / default-password flow keeps working unchanged. Purely additive.
MinerAuthalready carries username/password; this adds an optional pre-issued token for firmwares that accept one (new(user, pass)unchanged,with_tokenbuilder).set_auth(username, password, token=None)exposes it.Happy to shape the API to your preference.