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

SSH Agent: Allow using database path to resolve keys #6365

Merged

Conversation

hifi
Copy link
Member

@hifi hifi commented Apr 4, 2021

This slightly breaks the encapsulation of KeeAgentSettings now depending on SSHAgent for the global configuration option but it's not too bad.

This has a minor header include conflict with #6209 and can wait for it to be merged first.

I will do a class forwards declaration cleanup later for all SSH Agent stuff so this continues the tradition of including the classes for now.

Closes #5225

Screenshots

Testing strategy

Type of change

  • ✅ New feature (change that adds functionality)

@hifi hifi added this to the v2.7.0 milestone Apr 4, 2021
@droidmonkey
Copy link
Member

Do we need an option for this? Other behavior in the app is that a relative path is always resolved based off the database location. We also have the {DB_DIR} placeholder.

@hifi
Copy link
Member Author

hifi commented Apr 4, 2021

KeeAgent uses the working directory according to their documentation so this is would be a compatibility breaking change to change the default behavior. I don't mind breaking it but someone probably would. Which approach we take?

@droidmonkey
Copy link
Member

droidmonkey commented Apr 4, 2021

What's the behavior if the setting is disabled? It just fails? Assumes absolute?

@hifi
Copy link
Member Author

hifi commented Apr 4, 2021

Relative to working directory. So pretty random but it is user controllable at will.

@droidmonkey
Copy link
Member

I have no problem breaking that behavior

@hifi hifi force-pushed the feature/sshagent-database-rel-path branch from 73189b5 to f78a73e Compare April 6, 2021 04:41
@hifi hifi marked this pull request as ready for review April 6, 2021 05:03
@hifi
Copy link
Member Author

hifi commented Apr 6, 2021

Removed the configuration options. I also believe it's more correct to be relative to the database by default.

@droidmonkey droidmonkey merged commit 9b8feed into keepassxreboot:develop Apr 7, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Allow relative path for external SSH keys
2 participants