There are types inaccessible to the authenticator role, such as
redsys_request, that can not be registered because that roles is unable
to get the OID of.
I could have moved these types to the public schema, but then it would
seem pointless to have a separate schema.
Every company need to have its own merchant code and encryption key,
thus it is not possible to use environment variables to keep that data,
and i have to store it in the database.
I do not want to give SELECT permission on the encryption key to guest,
because i am going to fuck it up sooner or later, and everyone would be
able to read that secret; i know it would. Therefore, i need a security
definer function that takes the data to encrypt, use the key to encrypt
it, and returns the result; nobody else should have access to that key,
not even admins!
By the way, i found out that every merchant receives the same key, thus
it is not a problem to keep it in the repository.
Since i need that SQL function to encrypt the data, i thought that i may
go the whole nine yards and sign the request in PostgreSQL too, after
all the data to sign comes from there, and it has JSON functions to
create and base64-code an object.
Fortunately, pg_crypto has all the functions that i need, but i can no
longer keep that extension inside the auth schema, because it is used
from others, and the public schema, like every other extensions, seems
more appropriate.
Instead of having the list of currency and language codes that Redsys
uses as constants in the code, i moved that as field to the currency
and language relations, so i can simply pass the lang_tag to the
function and it can transform that tag to the correct code; the currency
is from the company’s relation, since it is the only currency used in
the whole application (for now).
As a consequence, i had to grant execute to currency and the parse_price
functions to guest, too.
To generate the test data used in the unit tests, i used a third-party
PHP implementation[0], but i only got from that the resulting base64-coded
JSON object and signature, using the same that as in the unit test, and
did not use any code from there.
PostgreSQL formats the JSON as text differently than most
implementations i have seen: it adds spaces between the key name and
the colons, and space between the value and the separating comma. The
first implementation used replace() to format the JSON as exactly as
the PHP implementation, so that the result matches, and then tried to do
generate the form by hand using the output from PostgreSQL without the
replace(), to verify that Redsys would still accept my input. Finally,
i adjusted the unit test to whatever pg_prove said it was getting from
the function.
I still have the form’s action hard-codded to the test environment, but
the idea is that administrators should be able to switch from test to
live themselves. That means that i need that info in the redsys
relation as well. I think this is one of the few use cases for SQL’s
types, because it is unlikely to change anytime soon, and i do not need
the actual labels.
Unfortunately, i could not use enumerations for the request’s
transaction type because i can not attach an arbitrary number to the
enum’s values. Having a relation is overkill, because i would need
a constant in Go to refer to its primary key anyway, thus i kept the
same constant i had before for that. Language and currency constant
went out, as this is in the corresponding relations.
In setup_redsys i must have a separate update if the encrypt_key is null
because PostgreSQL checks constraints before key conflict, and i do
not want to give a default value to the key if the row is not there yet.
The problem is that i want to use null to mean “keep the same password”,
because it is what i intend to do with the user-facing form: leave the
input empty to keep the same password.
As now Go needs to pass composite types back and forth with PostgreSQL,
i need to register these types, and i have to do it every time because
the only moment i have access to the non-pooled connection is in the
AfterConnect function, but at that point i have no idea whether the
user is going to request a payment. I do not know how much the
performance degrades because of this.
[0]: https://github.com/ssheduardo/sermepa/blob/master/src/Sermepa/Tpv/Tpv.php
This is the image that is shown at the home page, and maybe other pages
in the future. We can not use a static file because this image can be
changed by the customer, not us; just like name and description.
I decided to keep the actual media content in the database, but to copy
this file out to the file system the first time it is accessed. This is
because we are going to replicate the database to a public instance that
must show exactly the same image, but the customer will update the image
from the private instance, behind a firewall. We could also synchronize
the folder where they upload the images, the same way we will replicate,
but i thought that i would make the whole thing a little more brittle:
this way if it can replicate the update of the media, it is impossible
to not have its contents; dumping it to a file is to improve subsequent
requests to the same media.
I use the hex representation of the media’s hash as the URL to the
resource, because PostgreSQL’s base64 is not URL save (i.e., it uses
RFC2045’s charset that includes the forward slash[0]), and i did not
feel necessary write a new function just to slightly reduce the URLs’
length.
Before checking if the file exists, i make sure that the given hash is
an hex string, like i do for UUID, otherwise any other check is going
to fail for sure. I moved out hex.Valid function from UUID to check for
valid hex values, but the actual hash check is inside app/media because
i doubt it will be used outside that module.
[0]: https://datatracker.ietf.org/doc/html/rfc2045#section-6.8
It is inside the “user menu” only because this is where Numerus has the
same option, although it makes less sense in this case, because Numerus
is geared toward individual freelancers while Camper is for companies.
But, since it is easy to change afterward, this will do for now.
However, it should be only shown to admin users, because regular
employees have no UPDATE privilege on the company relation. Thus, the
need for a new template function to check if the user is admin.
Part of #17.
I really doubt that they are going to use more than a single company,
but the application is based on Numerus, that **does** have multiple
company, and followed the same architecture and philosophy: use the URL
to choose the company to manage, even if the user has a single company.
The reason i use the slug instead of the ID is because i do not want to
make the ID public in case the application is really used by employees
of many unrelated companies: they need not need to guess how many
companies there are based on the ID.
I validate this slug to be a valid UUID instead of relaying on the
query’s empty result because casting a string with a malformed value to
UUID results in an error other than data not found. Not with that
select, but it would fail with a function parameter, and i want to add
that UUID check to all functions that do use slugs.
I based uuid.Valid function on Parse() from Google’s uuid package[0]
instead of using regular expression, as it was my first idea, because
that function is an order of magnitude faster in benchmarks:
goos: linux
goarch: amd64
pkg: dev.tandem.ws/tandem/numerus/pkg
cpu: Intel(R) Core(TM) i5-6200U CPU @ 2.30GHz
BenchmarkValidUuid-4 36946050 29.37 ns/op
BenchmarkValidUuid_Re-4 3633169 306.70 ns/op
The regular expression used for the benchmark was:
var re = regexp.MustCompile("^[a-fA-F0-9]{8}-[a-fA-F0-9]{4}-4[a-fA-F0-9]{3}-[8|9|aA|bB][a-fA-F0-9]{3}-[a-fA-F0-9]{12}$")
And the input parameter for both functions was the following valid UUID,
because most of the time the passed UUID will be valid:
"f47ac10b-58cc-0372-8567-0e02b2c3d479"
I did not use the uuid package as is, even though it is in Debian’s
repository, because i only need to check whether the value is valid,
not convert it to a byte array. As far as i know, that package can not
do that.
Adding the Company struct into auth was not my intention, as it makes
little sense name-wise, but i need to have the Company when rendering
templates and the company package has templates to render, thus using
the company package for the Company struct would create a dependency
loop between template and company. I’ve chosen the auth package only
because User is also there; User and Company are very much related in
this application, but not enough to include the company inside the user,
or vice versa, as the User comes from the cookie while the company from
the URL.
Finally, had to move methodNotAllowed to the http package, as an
exported function, because it is used now from other packages, namely
campsite.
[0]: https://github.com/google/uuid
Conceptually, to logout we have to “delete the session”, thus the best
HTTP verb would be `DELETE`. However, there is no way to send a
`DELETE` request with a regular HTML form, and it seems that never will
be[0].
I could use a POST, optionally with a “method override” technique, but
i was planing to use HTMx anyway, so this was as good an opportunity to
include it as any.
In this application i am not concerned with people not having JavaScript
enabled, because it is for a customer that has a known environment, and
we do not have much time anyway. Therefore, i opted to forgo
progressive enhancement in cases like this: if `DELETE` is needed, use
`hx-delete`.
Unfortunately, i can not use a <form> with a hidden <input> for the
CSRF token, because `DELETE` requests do not have body and the value
should be added as query parameters, like a form with GET method, but
HTMx does the incorrect thing here: sends the values in the request’s
body. That’s why i have to use a custom header and the `hx-header`
directive to include the CSRF token.
Then, by default HTMx targets the triggered element for swap with the
response from the server, but after a logout i want to redirect the
user to the login form again. I could set the hx-target to button to
replace the whole body, or tell the client to redirect to the new
location. I actually do not know which one is “better”. Maybe the
hx-target is best because then everything is handled by the client, but
in the case of logout, since it is possible that i might want to load
scripts only for logged-in users in the future, i opted for the full
page reload.
However, HTMx does not want to reload a page that return HTTP 401,
hence i had to include the GET method to /login in order to return the
login form with a response of HTTP 200, which also helps when
reloading in the browser after a failed login attempt. I am not worried
with the HTTP 401 when attempting to load a page as guest, because
this request most probably comes from the browser, not HTMx, and it will
show the login form as intended—even though it is not compliant, since
it does not return the WWW-Authenticate header, but this is the best i
can do given that no cookie-based authentication method has been
accepted[1].
[0]: https://www.w3.org/Bugs/Public/show_bug.cgi?id=10671#c16
[1]: https://datatracker.ietf.org/doc/id/draft-broyer-http-cookie-auth-00.html
To get the user from the database i have to set the cookie first, that
was already done in database.MustAcquire, but i thought they were too
far apart, even thought they are so related. So, the cookie, and thus
the role, is set when getting the user, that is actually the first thing
to do once the connection is acquired. However, that way the database
package has no knowledge of cookies, and the code that sets the cookie
and retrieves the user are next to each other.
I applied the same logic to the changes of locale.Match: it has not
business knowing that the accept language string comes from a request;
it only needs the actual string. Also, the TODO comment about getting
the user’s locale made no sense, now, because app already knows that
locale, so there is no need to pass the user to the locale package.
Getting the locale is done after retrieving the user from the database,
for the same reason the connection is Acquired as far up as possible:
almost every request will need this value, together with the user and
the database connection.
I am a bit affraid that i will end up with functions that always expect
these three values. Maybe i can put the locale inside user, as it is
the user’s locale, after all, no matter if it came from the database or
the user agent, but connection and user must be separate, i think.
We’ll see.
Almost all request will need the database connection, either because
they will perform queries or because they need to check if the user is
logged in, for instance, so it should be acquired as far above as
possible to avoid acquiring multiple connections.
This is specially true since i have to pass the cookie to the database
to switch role and set request.user.email and request.user.cookie
config variables. I do not want to do that many times per request.
I now actually handle the /login URL and check whether the email and
password are valid, creating the session cookie if correct, but doing
nothing else with that cookie, for now.
The validation is done by hand for now, because i do not yet how i will
actually do it without so much duplication.
It does nothing more than to server a single page that does nothing
interesting.
This time i do not use a router. Instead, i am trying out a technique
i have seen in an article[0] that i have tried in other, smaller,
projects and seems to work surprisingly well: it just “cuts off” the
URI path by path, passing the request from handler to handler until
it finds its way to a handler that actually serves the request.
That helps to loosen the coupling between the application and lower
handlers, and makes dependencies explicit, because i need to pass the
locale, company, etc. down instead of storing them in contexts. Let’s
see if i do not regret it on a later date.
I also made a lot more packages that in Numerus. In Numerus i actually
only have the single pkg package, and it works, kind of, but i notice
how i name my methods to avoid clashing instead of using packages for
that. That is, instead of pkg.NewApp i now have app.New.
Initially i thought that Locale should be inside app, but then there was
a circular dependency between app and template. That is why i created a
separate package, but now i am wondering if template should be inside
app too, but then i would have app.MustRenderTemplate instead of
template.MustRender.
The CSS is the most bare-bones file i could write because i am focusing
in markup right now; Oriol will fill in the file once the application is
working.
[0]: https://blog.merovius.de/posts/2017-06-18-how-not-to-use-an-http-router/
jordi fita mas