Skip to content

Store api tokens securely in the macOS Keychain#62

Open
lindenstruth wants to merge 1 commit intoamiantos:masterfrom
lindenstruth:master
Open

Store api tokens securely in the macOS Keychain#62
lindenstruth wants to merge 1 commit intoamiantos:masterfrom
lindenstruth:master

Conversation

@lindenstruth
Copy link

@lindenstruth lindenstruth commented Jun 23, 2025

Pibar is now storing api tokens securely in the macOS Login Keychain instead of using the UserDefaults for plain text storage. For this purpose an additional SPM dependency for a 3rd party keychain wrapper has been added.

The way it is implemented, existing configurations should automatically be updated. This has the imperfection of a few keychain access requests too many during app launch, but the advantage of being fully transparent and requiring a minimal amount of code change.

I haven't been able to test it with multiple piholes yet due to the lack of available piholes (I have only a single one up and running so far).

Pibar is now storing api tokens securely in the macOS Login Keychain instead of using the UserDefaults for plain text storage. For this purpose an additional SPM dependency for a 3rd party keychain wrapper has been added.
@amiantos
Copy link
Owner

amiantos commented Jun 23, 2025

Some quick thoughts...

It feels like it would make more sense for this stuff to go into Preferences.swift, to get everything related to Pi-hole connection storage isolated into one place. But it would probably necessitate some bigger changes to how connections are stored and initialized, adding possibly another 'migration' path. Or maybe it could just be slipped in there similar to what is going on here.

Keychain API is also not super complicated so it would be nicer to handle all the code for that in-house, similar to Postalgic, instead of using an external dependency for it.

@lindenstruth
Copy link
Author

it was the quickest way for me to implement it, since I've already used this keychain wrapper before, and the changes to the codebase were minimal. But I'd be happy with any other solution as well that stores the token in the keychain rather than the UserDefaults. Come to think of it: I believe I forgot to implement deleting the token from the keychain again when removing a pihole config 🫣

@amiantos
Copy link
Owner

I'm fine with merging this in for now and I can swap it around as I make changes for the real 1.2 release, I am probably going to rewrite a bunch of stuff for that anyway.

Copy link
Owner

@amiantos amiantos left a comment

Choose a reason for hiding this comment

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

LGTM, when I work on the next/final 1.2 release I'll move this around somewhere else.

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.

2 participants