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

Allow accessing frigate with authentication #801

Merged

Conversation

loispostula
Copy link
Contributor

Fix #697

@dermotduffy dermotduffy added the enhancement New feature or request label Dec 7, 2024
@dermotduffy
Copy link
Collaborator

Please ensure the tests pass, then I'll take a look, thanks!

Copy link

codecov bot commented Dec 7, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 100.00%. Comparing base (a95d327) to head (e71aaac).
Report is 14 commits behind head on master.

Additional details and impacted files
@@            Coverage Diff            @@
##            master      #801   +/-   ##
=========================================
  Coverage   100.00%   100.00%           
=========================================
  Files           17        17           
  Lines         2077      2140   +63     
=========================================
+ Hits          2077      2140   +63     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

Copy link
Owner

@blakeblackshear blakeblackshear left a comment

Choose a reason for hiding this comment

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

Looks good to me. Thanks for this.

tests/test_api.py Outdated Show resolved Hide resolved
@dermotduffy
Copy link
Collaborator

@loispostula This is fantastic, thank you. Few comments added.

@dermotduffy
Copy link
Collaborator

You probably also need to update en.json at least for string translations of the new parameters. It would be good to include the word "optional" in the string description for both.

@loispostula loispostula force-pushed the lp/authenticated_frigate branch from 2cd903b to 920e15d Compare December 8, 2024 14:55
@loispostula
Copy link
Contributor Author

@dermotduffy thank you for your review, I've rebased the PR to incorporate the reconfigure changes, it's unclear to me what change would be needed to allow the reconfiguration of the username and password. It seemed to me that no changes are required for that to happen.

Also I'm not familiar with ha translation, so I can add the translation for username and password, but not sure how to hook that from the en.json to the config_flow

Copy link
Collaborator

@dermotduffy dermotduffy left a comment

Choose a reason for hiding this comment

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

  • For reconfigure, yes, now that it's rebased, I think this will "just work"!
  • For translations, you'll just need to edit en.json and add new entries for
        "data": {
          "url": "URL",
          "username": "Username (optional)",
          "password": "Password (optional)",
        }

... then to test, when you restart HA with this, you should see it show those strings next to those fields. Don't worry about other languages, unless you happen to speak them ;-)

custom_components/frigate/__init__.py Outdated Show resolved Hide resolved
custom_components/frigate/api.py Outdated Show resolved Hide resolved
custom_components/frigate/config_flow.py Outdated Show resolved Hide resolved
@dermotduffy dermotduffy merged commit 0cced70 into blakeblackshear:master Dec 11, 2024
6 checks passed
@dermotduffy
Copy link
Collaborator

Thanks @loispostula !

@los93sol
Copy link

Was this based on my #753

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Support Authentication WebUI introduced in Frigate 0.14.0
5 participants