PyOTP's totp.verify defaults to the valid_window of zero, which means
it will reject valid codes, if submitted just past the 30 sec window.
It also means, users will run into authentication issues very quickly
if their phones time-sync isn't perfect.
Therefore valid_window should at the very least be 1 or more, settting
it higher trades security for robustness, especially with regard to
time desync issues.
Resolves the following issue, which occurs with force_otp enabled
and OAuth authentication sources:
File "/srv/powerdnsadmin/powerdnsadmin/models/user.py", line 481, in update_profile
"utf-8") if self.plain_text_password else user.password
AttributeError: 'User' object has no attribute 'plain_text_password'
Allow the new domain name to be input absolute (with a dot at the end).
To keep the rest of the logic working as-is, remove it fairly early in
the function.
Would have loved to use `str.removesuffix()` but that's python v3.9+.
When clicking the changelog button for a record with the name
`foo-bar.example.org`, the url you get redirected to is
`/domain/example.org/changelog/foo-bar.example.org.-A`. Because of the
non-greedy behaviour of the path converter, the last part gets split at
the *first* hyphen, so the example above gets wrongly dissected into
`record_name=foo` and `record_type=bar.example.org.-A`. This results
for obvious reasons in an empty changelog.
As described in rfc5395 [0], types have to be alphanumerical, so its
converter is changed from path to string.
The hyphen is one of the few characters recommended by rfc1035 [1],
so it is a bad choice as separator. The separator is instead changed to
a slash.
Granted, this does not entirely solve the issue but at least makes it a
lot less likely to happen. Plus, a lot more and other things break in
pda with slashes in names.
[0] https://datatracker.ietf.org/doc/html/rfc5395#section-3.1
[1] https://datatracker.ietf.org/doc/html/rfc1035#section-2.3.1
Moved all the logic out of the template into a separate endpoint. This
makes it easy to extend to also support images from different sources
like LDAP/SAML/OIDC. Session-based caching is hard to do, so to allow
time-based caching in the browser, the url needs to be unique for every
user by using a query parameter.
Replaced the default/fallback user image with a new one. It is based on
the old one, but does not need css to be visible. And removed said css.
Gravatar has now its own setting named `gravatar_enabled`, which is
disabled by default.
Starting with the very first commit, the update was always done with
two api calls: one for DELETE and one for REPLACE. It is however
perfectly valid and save to do both at once, which makes it atomic, so
no need for the rollback. Plus it only updates the serial once.
There is no point in sending the full RRset data when deleting it, the
key attributes to identify it are enough. This also make the behaviour
consistent with the api docs [0] where it says "MUST NOT be included
when changetype is set to DELETE."
[0] https://doc.powerdns.com/authoritative/http-api/zone.html#rrset
Setting this attribute on a cookie marks it as non-cross-site, so it
is only send in requests to our own server. It is reasonable that no
one else should need our session or csrf data. Setting it explicitly
also prevents any issues from the ongoing change in browser behaviour [0]
when it is unset.
Seasurf supports the SameSite attribute starting with v0.3. As nothing
obviously broke, I used the opportunity and updated all the way to the
most recent version.
The SeaSurf default for SameSite is already `Lax`, so it only needs to
be set for the session cookie.
[0] https://developers.google.com/search/blog/2020/01/get-ready-for-new-samesitenone-secure
CSRF has been initialized *before* the app config was fully read. That
made it impossible to configure CSRF properly. Moved the CSRF init into
the routes module, and switched from programmatic to decorated
exemptions. GET routes don't need to be exempted because they are by
default.
When enabled, forbids the creation of a domain if it exists as a record in one of its parent domains (administrators and operators are not limited though).
There is a misspelling of rrset throughout the history logic, which also
effects the json payload in the database. Code-wise this is a simple
search-and-replace, and the migration will fix the payloads.
Semantically and syntactically it is better to have the same number of
`<th>` as `<td>`. Not that anyone will ever see that new header, since
that column is always invisible (except if the user disables javascript).
Plus remove a unmatched closing html element.
As vermin [0] confirms, the codebase has long moved beyond supporting
python v2 (which is not a bad thing). This removes the last explicit py2
piece of code.
And in case anyone wonders, vermin currently reports the minium version
to be v3.6.
[0] https://pypi.org/project/vermin/
* Previously having characters like "ü" in the SOA wouldnt allow to push
updates to the domain
* Also use the new method to_idna to support characters like "ß"
Previously strings with characters like "ß" would throw and exception
This seems to happen because the lib behind encode().decode('idna')
cant handle characters like this
- Store HTML for modal window inside an invisible <div> element instead
of inside the <button> element's value attribute
- Mark history.detailed_msg as safe as it is already manually run
through the template engine beforehand and would be broken if escaped
a second time
If the 'otp_force' and 'otp_field_enabled' basic settings are both enabled, automatically enable 2FA for the user after login or signup, if needed, by setting a new OTP secret. Redirect the user to a welcome page for scanning the QR code.
Also show the secret key in ASCII form on the user profile page for easier copying into other applications.
- Run HTML through the template engine, preventing XSS from various
vectors
- Fix uncaught exception when a history entry about domain template
deletion is processed
- Adapt indentation to 4 space characters per level
The order of account names returned by User.get_accounts() affects the
order account names are displyed in on /domain/add if the current user
neither has the Administrator role nor the Operator role and the
`allow_user_create_domain` setting is enabled at the same time.
If the current user does have the Administrator or Operator role,
routes.domain.add() already returns accounts ordered by name, so this
change makes it consistent.
The implementation of `random.choice()` uses the Mersenne Twister, the
output of which is predictable by observing previous output, and is as
such unsuitable for security-sensitive applications. A cryptographically
secure pseudorandom number generator - which the `secrets` module relies
on - should be used instead in those instances.
When creating a new local user, there is a chance that, due to a copy &
paste or typing error, whitespace will be introduced at the start or end
of the username. This can lead to issues when trying to log in using the
affected username, as such a condition can easily be overlooked - no
user will be found in the database if entering the username without the
aforementioned whitespace. This commit therefore strip()s the username
string within routes/{admin,index}.py.
The firstname, lastname and email strings within
routes/{admin,index,user}.py are also strip()ped on this occasion.
The common procedure for HTTP Basic Auth is that a client does /not/
immediately send out credentials via an 'Authorization'-header, but to
wait until the server tells the client to do so - which the server
indicates via the 'WWW-Authenticate'-header.
PowerDNS-Admin (and flask in general), though, abort the whole
communication if no Authorization header was found in the initial
request - resulting in '200 "badauth"'.
While this might work for /some/ HTTP clients - which right away add an
Authorization header crafted from provided credentials (via args or
extracted from given URL), this is /not/ standard and /not/ common.
Hence add the 'WWW-Authenticate'-header for every unauthenticated call
checking for dyndns authorisation.
Note, though, this changes the status code from 200 to 401 in this case,
which - given the explanation why 200 was chosen in the first place -
might cause side effects.
The onelogin package is not part of all saml packages for whatever
reason (e.g. Debian) and not easily installable from pypi (requires
CC toolchain).
As the onelogin functionality is already guarded by whether
SAML_ENABLED is set in other places (services/saml.py), also do so
in routes/index.py.
* Fix typo in managing user account membership with SAML assertion
* Support more config options from Docker env.
* Improve support for SAML key and cert from Docker secrets
Co-authored-by: Ian Bobbitt <ibobbitt@globalnoc.iu.edu>
As the URL here is contructed from Flask's "url_for" which already
takes the script root into account, we do not need to add it here
explicitly. This would result in a duplicate script root otherwise.
This increases data inconsistency and also performance of the whole
update process: SqlAlchemy can use its own caching better with a single
transaction as it does not need to mark already fetched objects as dirty
and so re-fetch them again on later use. So after deleting a domain,
we can still use the previously fetched domain objects for the update
operations afterwards. Otherwise SqlAlchemy had to fetch each domain
object again with single SELECT statements which leads to bad
performance with many domains (>> 1000).
The introduced "do_commit" variable controls whether the database
changes are to be committed by the caller or in the called method
itself. So we can use a single transaction for the Domain.update()
method while still preserving the previous behavior for other callers.
Closes#428.
The new setting 'verify_ssl_connections' tells the requests library to
verify secured outgoing HTTP connections.
Usually verifying is desired and helps to reveal configuration
problems. It also disables an ugly warning when HTTPS connections
are made without verification.
Enhancements:
- More robust check when creating self-signed certificates
- Added support for SAML Requested Attributes through "SAML_SP_REQUESTED_ATTRIBUTES" parameter
Problems resolved:
- Method create_self_signed_cert() was invoked nowhere. This puts parameter "SAML_SIGN_REQUEST" description in configs/development.py as incorrect
- Method create_self_signed_cert() was returning error while trying to write out certificate and private key. File handler was opened for writing out TEXT instead of BINARY data
Enhancements:
- Two new parameters are introduced SAML_CERT_FILE and SAML_KEY_FILE. User can now explicitly define own certificate and key file anywhere on file-system.
- If parameters mentioned in previous bullet aren't explicitly defined, in PowerDNS-Admin root directory self-signed certificate will be created.
- Certificates will be used or generated in any case, because in saml.py there are explicit parameters defined which require certificate/key in order to work normally. If they aren't, exception will be thrown. Examples of parameters defined in saml.py requiring certificate: wantAssertionsEncrypted, signMetadata, wantAssertionsSigned.
- Add session handler on other blueprint's before request
- Adjustment in using jTimeout to close warning popup on
other tabs when we extend the session
From my perspective, if agreed, this change can be merged, because the basic SAM auth. functionality is now present
and was tested with "samlidp.io" iDP.
However, there are further improvements which I would like to integrate, but as a separate features in separate pull requests
- Function signatures inside SAML class were fixed
- Redirect URL for /saml/login path was modified (saml_authorized -> index.saml_authorized)
Current status is that SAML metadata can be generated under /saml/metadata and communication to SAML iDP is working
Problems remaining:
- SAML Response doesn't contain any attributes (There is no AttributeStatement on the Response). It can be that problem is on iDP side
- Background thread in retrieve_idp_data() cannot be spawned, this part is currently commented out, old code needs to be revisited
