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

Clean up client identity configuration options #107

Open
tlater-famedly opened this issue Jan 22, 2025 · 0 comments
Open

Clean up client identity configuration options #107

tlater-famedly opened this issue Jan 22, 2025 · 0 comments
Labels
tech debt Work on this should be planned according to tech debt policy Technical Debt This Issue is a technical debt. Mark issues with this label to have an overview of the code quality.

Comments

@tlater-famedly
Copy link
Contributor

tlater-famedly commented Jan 22, 2025

We currently have a fair bit of awkward code to handle tls certificates in the ldap source config:

let identity: Option<Identity> = match (tls.client_key, tls.client_certificate) {
(Some(client_key), Some(client_cert)) => Some(
Identity::from_pkcs8(
std::fs::read(client_cert)?.as_slice(),
std::fs::read(client_key)?.as_slice(),
)
.context("Could not create client identity")?,
),
(None, None) => None,
_ => {
bail!("Both client key *and* certificate must be specified")
}
};

In retrospect, we should bundle these in a client_identity attribute which is ~Option<struct ClientIdentity(PathBuf, PathBuf)> . That way we can assert at the type level that both are specified at the same time, and give a cleaner error message (directly from serde, so that the attribute and everything is listed) as a result.

That'd require a breaking change, unfortunately.

Originally posted by @tlater-famedly in #106 (comment)

@tlater-famedly tlater-famedly added Technical Debt This Issue is a technical debt. Mark issues with this label to have an overview of the code quality. tech debt Work on this should be planned according to tech debt policy labels Jan 22, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
tech debt Work on this should be planned according to tech debt policy Technical Debt This Issue is a technical debt. Mark issues with this label to have an overview of the code quality.
Projects
None yet
Development

No branches or pull requests

1 participant