Skip to content

fix(security): use keyring to store tokens upon login#123

Merged
rawnly merged 2 commits intomainfrom
feature/keyring
May 17, 2025
Merged

fix(security): use keyring to store tokens upon login#123
rawnly merged 2 commits intomainfrom
feature/keyring

Conversation

@rawnly
Copy link
Owner

@rawnly rawnly commented May 16, 2025

No description provided.

@rawnly rawnly requested a review from Copilot May 16, 2025 13:49
@rawnly rawnly self-assigned this May 16, 2025
Copy link

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull Request Overview

This PR enhances security by migrating token storage from local config (using Viper) to the OS keyring.

  • Introduces a new tokens package leveraging go-keyring to set, get, and clear tokens.
  • Updates API utilities, commands, and main.go to use keyring-based token operations instead of Viper.
  • Removes sensitive debug logging of client credentials in the Me() method.

Reviewed Changes

Copilot reviewed 9 out of 9 changed files in this pull request and generated 3 comments.

Show a summary per file
File Description
unsplash/util.go Switched from viper.GetString to tokens.GetAccessToken and simplified debug logs.
unsplash/tokens/tokens.go New package to store, retrieve, and clear access/refresh tokens via keyring.
unsplash/auth.go Removed debug logging of client_id and client_secret.
main.go Replaced Viper-based token retrieval with keyring-based tokens.Get*.
cmd/auth/whoami.go Updated to retrieve the access token from keyring.
cmd/auth/logout.go Added tokens.Clear() before clearing Viper entries.
cmd/auth/login.go Switched to tokens.Set* for storing tokens and fixed authenticationURL naming.
go.mod Added go-keyring and updated indirect dependencies for keyring support.

Run: func(cmd *cobra.Command, args []string) {
fmt.Println("Logging out...")

tokens.Clear()
Copy link

Copilot AI May 16, 2025

Choose a reason for hiding this comment

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

After clearing tokens in the keyring, the Viper config is set but not persisted; add viper.WriteConfig() to ensure the config file no longer retains auth tokens.

Copilot uses AI. Check for mistakes.
@rawnly rawnly merged commit c39ae9c into main May 17, 2025
7 checks passed
@rawnly rawnly deleted the feature/keyring branch May 17, 2025 10:54
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants