ocaml-uri
ocaml-uri copied to clipboard
Pretty-printer with redacted/hidden passwords
Hiding passwords in a pretty-printer is useful for logging. It may be worth having passwords hidden by default, with a way to opt-in to password printing.
A few possible options:
(* Fields we can redact when printing *)
type field =
| Username
| Password
| Query of [`Full | `Values]
| And_so_on
(* Opt-out of printing specific fields, replacing them with generic filler text.
[redact] could default to [[Username; Password; Query `Values]] in a
paranoid world. *)
val pp_redacted : ?redact:field list -> unit -> t Fmt.t
(* Opt-in to password printing - with_password would default to [false]. *)
val pp_passwordless : t Fmt.t
(* Just strip out everything *)
val pp_paranoid : t Fmt.t
I agree we should redact the passwords by default.
Should we change the behavior of pp and add a separate, more customizable pretty printer as well? I'd be happy to make a PR for this once an approach is decided on.
Given the existence of GDPR, the best approach is probably to default to not printing the username and password in pp. How about just a single function with the signature of pp_redacted above with a ?redact=[Username;Password] default?
I started looking at this a bit and have a few API design questions/thoughts.
I think `User and `Password constructors should be added to the component type rather than adding the redundant field type I suggested in the original issue text.
Rather than adding a pp_redacted function, I think we should make the components to redact a global ref and always use that ref's contents to determine the printing behavior in pp. So pp would act like pp_redacted as originally described but ?redact would be pulled from a (spooky!) global ref rather than a function argument.
As pure evil as global references may be, the ref-based approach does have the benefit of being accessible without having to grep/sed for all potential uses of Uri.pp.
The interface additions would be:
(** `User and `Password would be added to the component type definition *)
(** {!pp}'s behavior would silently change to pull from the set of redacted components when printing *)
(** Set of redacted components when printing with {!pp}. Can be changed with {!add_redacted} and {!remove_redacted}. Defaults to [`User] and [`Password]. *)
val redacted : unit -> component list
(** [add_redacted component] adds [component] to the set of redacted components when printing with {!pp}. *)
val add_redacted : component -> unit
(** [remove_redacted component] removes [component] from the set of redacted components when printing with {!pp}. *)
val remove_redacted : component -> unit
It would be better to mark Uri.pp as deprecated; then the compiler can find all the uses for you. Different callers will need different parts redacted, so having a global for this doesn't seem useful. For example, when a program prints out a message on first run telling you to visit an admin URL to configure the server, the URL printed must include all secret components.