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

Support both Digest and API key authentication #25

Closed
wants to merge 10 commits into from

Conversation

john-rodewald
Copy link

I would like to integrate my Prusa SL1S into Home Assistant. Prusa supports both Digest and API key authentication but recommends Digest. Thus I've updated both pyprusalink and the corresponding Home Assistant component in home-assistant/core to allow for this.

The API change is breaking.

I intend to keep working on this for my local setup, primarily adding sensors that are useful for resin printers. Maybe you're interested in upstreaming. If so, I'll also open a PR against home-assistant/core.


I currently know of one open problem I could use help with if you'd like:

  • The HASS component checks if the API version is 2 and refuses to connect to the printer otherwise. But my SL1S actually returns the following from /api/version:
{
    "api": "0.1",
    "hostname": "prusa-sl1s",
    "server": "1.1.0",
    "text": "Prusa SLA 1.0.5"
}

Do you have any idea why that is? Is your printer running "api": "2.0"?

@@ -0,0 +1,13 @@
{ pkgs ? import <nixpkgs> {} }:
Copy link
Contributor

Choose a reason for hiding this comment

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

We should not include nix configuration in the package.

pyproject.toml Show resolved Hide resolved
@balloob
Copy link
Contributor

balloob commented Jan 2, 2023

If the SL1S returns API version 0.1 and uses a different auth scheme, it's not the right version of the prusalink API? What other things are incompatible?

Are they going to upgrade SL1S for the new prusalink?

@john-rodewald
Copy link
Author

john-rodewald commented Jan 2, 2023

If the SL1S returns API version 0.1 and uses a different auth scheme, it's not the right version of the prusalink API? What other things are incompatible?

Are they going to upgrade SL1S for the new prusalink?

The SL1S is not incompatible with the old scheme per se - but Prusa officially recommends Digest over using API keys. At least such a warning is displayed on the SL1S. It seemed sensible to me to support both methods in pyprusalink.

I can't yet say for certain why API version 0.1 is returned on my printer. The latest updates are installed on it already and the API calls made by pyprusalink all work as expected. It's as though API v2 was in fact running on it.

@balloob
Copy link
Contributor

balloob commented Jan 2, 2023

Should we start with opening an issue for the version number on https://github.com/prusa3d/Prusa-Firmware-SL1 for that? That sounds like a bug.

The SL1S is not incompatible with the old scheme per se - but Prusa officially recommends Digest over using API keys.

What you're saying is that it does support API keys?

@john-rodewald
Copy link
Author

Should we start with opening an issue for the version number on https://github.com/prusa3d/Prusa-Firmware-SL1 for that? That sounds like a bug.

Possibly! I’ll check if it’s a known issue at all.

What you're saying is that it does support API keys?

Yup, the SL1S supports both.

@balloob
Copy link
Contributor

balloob commented Jan 3, 2023

In that case I don't see a reason to change from aiohttp to httpx just to get digest working. They only support API keys on the mini so it doesn't look like it's the recommended way.

@balloob balloob closed this Jan 3, 2023
@john-rodewald
Copy link
Author

john-rodewald commented Jan 3, 2023

In that case I don't see a reason to change from aiohttp to httpx just to get digest working. They only support API keys on the mini so it doesn't look like it's the recommended way.

On the Prusa SL1S, Digest is in fact explicitly recommended as I’ve stated above. With that in mind, do you still wish to stop moving forward with this?

@mathieucarbou
Copy link

@balloob : we need digest auth C an this be re-opened ?

@balloob
Copy link
Contributor

balloob commented Mar 20, 2023

Maybe I am missing something, but how do you know that is the fix for your issue?

@mathieucarbou
Copy link

mathieucarbou commented Mar 20, 2023

Maybe I am missing something, but how do you know that is the fix for your issue?

Please read:

The plugin is not working since PrusaLink 0.7.0rc2.

rom PrusaLink for MK3 0.7.0, API Key will be deprecated and not configured by default, but you can set it on Settings tab. New way to using PrusaLink API is HTTP Digest.

What I am unsure about is that a curl call works:

curl -H "X-Api-Key: xxxxxxxxxx" http://prusalink.local/api/v1/status

But the HA plugin is not able to communicate through API.

Error in HA:
image

I think this is because of this check: https://github.com/home-assistant/core/blob/dev/homeassistant/components/prusalink/config_flow.py#L49

For the 2 latest RC version, Prusalink returns:

{
  "api": "0.9.0-legacy",
  "server": "0.7.0rc3",
  "original": "PrusaLink I3MK3S",
  "text": "PrusaLink 0.7.0rc3",
  "firmware": "3.11.0-4955",
  "sdk": "0.7.0rc3",
  "capabilities": {
    "upload-by-put": true
  },
  "hostname": "PrusaLink"
}

@john-rodewald
Copy link
Author

john-rodewald commented Mar 20, 2023

What I am unsure about is that a curl call works:

curl -H "X-Api-Key: xxxxxxxxxx" http://prusalink.local/api/v1/status

But the HA plugin is not able to communicate through API.

I also get an interesting API version (0.1) from my SL1S when querying for it:
prusa3d/Prusa-Firmware-SL1#78
I've found no explanation as to why that is the case.

There exists a check in home-assistant/core that checks if the API version is >2:
https://github.com/home-assistant/core/blob/d33a303a83bc813c8abc35cb755e89773afb5e2d/homeassistant/components/prusalink/config_flow.py#L49

In both of our cases, the error seems to be raised there.

@mathieucarbou
Copy link

I agree. I saw the same thing. Let's continue the discussion here then: home-assistant/core#86230

@balloob
Copy link
Contributor

balloob commented Mar 20, 2023

This post seems like an official statement that API key is deprecated and this PR is ok to move forward: https://forum.prusa3d.com/forum/postid/634572/

@balloob balloob reopened this Mar 20, 2023
Comment on lines +4 to +5

This fork adds support for Digest authentication - the method Prusa recommends for their resin printers (SL1, SL1S).
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
This fork adds support for Digest authentication - the method Prusa recommends for their resin printers (SL1, SL1S).

@@ -94,53 +74,65 @@ async def pause_job(self) -> None:
async def get_version(self) -> VersionInfo:
"""Get the version."""
async with self.request("GET", "api/version") as response:
return await response.json()
return response.json()
Copy link
Contributor

Choose a reason for hiding this comment

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

This is very wrong, you're doing I/O in the event loop and hating Home Assistant. Use the async version of httpx.

Copy link
Contributor

Choose a reason for hiding this comment

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

or is httpx reading the whole response right away?

from .const import API_KEY_AUTH, DIGEST_AUTH


class VersionInfo(TypedDict):
Copy link
Contributor

Choose a reason for hiding this comment

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

Since there are multiple versions of the API that we all want to support, we should probably include a version number in the type names.


files: list[FileInfo]


class PrusaLink:
Copy link
Contributor

Choose a reason for hiding this comment

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

Do we need to split this into multiple classes, one for each version of the API? We can put the request function into a single base class.


files: list[FileInfo]


class PrusaLink:
"""Wrapper for the Prusalink API.

Data format can be found here:
Copy link
Contributor

Choose a reason for hiding this comment

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

Update this doc string to also include other APIs

@Skaronator
Copy link
Contributor

Superseded by #63

@balloob balloob closed this Nov 27, 2023
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.

4 participants