mostly stylistic nits inline, but also a comment w.r.t. FWICT needles ABI
breakage.
Am 23/10/2023 um 15:18 schrieb Folke Gleumes:
> implementation acording to rfc855 section 7.3.4
s/acording/according/
>
> Signed-off-by: Folke Gleumes <f.gleu...@proxmox.com>
> ---
> src/PVE/ACME.pm | 43 +++++++++++++++++++++++++++++++++++--------
> 1 file changed, 35 insertions(+), 8 deletions(-)
>
> @@ -251,6 +251,28 @@ sub jws {
> };
> }
>
> +# EAB signing using the HS256 alg (HMAC/SHA256).
At least write out the acronym "external account binding" out once here,
or maybe expand the method name "external_account_binding", for clarity.
> +sub eab {
> + my ($self, $eab_kid, $eab_hmac_key, $url) = @_;
> +
> @@ -329,21 +351,26 @@ sub __new_account {
> return $self->{account};
> }
>
> -# Create a new account using data in %info.
> +# Create a new account using data in $info.
> # Optionally pass $tos_url to agree to the given Terms of Service
> # POST to newAccount endpoint
> # Expects a '201 Created' reply
> # Saves and returns the account data
> sub new_account {
> - my ($self, $tos_url, %info) = @_;
> + my ($self, $tos_url, $info) = @_;
This needlessly breaks older call sites though? Why not keep the plain hash
here, you do not really win anything by switching to a hash ref, or?
Let's avoid churn w.r.t. breaking older package dependencies, if not really
required..
> my $url = $self->_method('newAccount');
>
> + if ($info->{'eab'}) {
nit: eab doesn't need quoting
And if you keep the method signature to accept a hash (not a hash reference)
here you could do something like:
if (my $eab = $info{eab}) {
my $eab_hmac_key = decode_base64($eab->{hmac_key});
# ..
}
> + my $eab_hmac_key = decode_base64($info->{'eab'}->{hmac_key});
nit: inconsistent hash key quoting in the same statement
> + $info->{externalAccountBinding} = $self->eab($info->{'eab'}->{kid},
> $eab_hmac_key, $url);
> + }
> +
> if ($tos_url) {
> $self->{tos} = $tos_url;
> - $info{termsOfServiceAgreed} = JSON::true;> +
> $info->{termsOfServiceAgreed} = JSON::true;
can be avoided by keeping the hash
> }
>
> - return $self->__new_account(201, $url, 1, %info);
> + return $self->__new_account(201, $url, 1, %{$info});
like above, can be avoided
> }
>
> # Update existing account with new %info
_______________________________________________
pve-devel mailing list
pve-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
