flake8
-
reviewboard/accounts/sso/backends/saml/sso_backend.py (Diff revision 1) Show all issues
Review Request #12254 — Created April 24, 2022 and submitted
Information | |
---|---|
david | |
Review Board | |
release-5.0.x | |
Reviewers | |
reviewboard | |
This change adds SAML 2.0 support for single-sign-on authentication. It
includes a few major pieces:
The vast majority of the settings available are specific to SAML,
including the various URLs and method settings. Because of some user
feedback we've already recieved, I've added a toggle to control the
behavior of user provisioning. In most cases, the person using this has
absolute trust in the integrity and correctness of their IdP, but in
some cases that may not be true. If the "username" parameter can not
necessarily be trusted, there's an option to force existing users to log
in with their Review Board password at least once.
Testing was done with a test application on onelogin.com with the
relevant configuration to authenticate to my server on localhost.
python3-saml
package wasn't installed.Summary | |
---|---|
Description | From | Last Updated |
---|---|---|
Looks good. Since this is WIP, I won't give it a Ship It!, but we're ready to move onto the … |
|
|
Do we need to do this, or can we rely on lazy population in the registry? |
|
|
Blank line between these. |
|
|
extra_data is worth having, but I think we may want to include something specific for credential data that we'd never … |
|
|
Missing docstring. |
|
|
Rather than this format, cna we do: for _module, _backend_cls_name in (('saml.sso_backend', 'SAMLSSOBackend'),): ... Or define the list separately. |
|
|
Missing docs in this file. |
|
|
This should be after __init__. |
|
|
F821 undefined name 'ModuleNotFoundError' |
![]() |
|
conf before contrib. |
|
|
404 before Response. |
|
|
These are in reverse order. |
|
|
These are in reverse order. |
|
|
Same with these. |
|
|
Missing docs. |
|
|
We should have some else case, even if that's just raising an exception. |
|
|
No need to .save() after .create(). |
|
|
Shouldn't have a trailing comma for a function call. |
|
|
Missing a Version Added. |
|
|
Missing a Version Added. We should put them in the functions as well. Same comments for other new modules. |
|
|
These should be swapped. |
|
|
Let's wrap any calls in an exception, in case any extension-provided backends (or ours) ever misbehave. |
|
|
Should we conditionally save ones with _enabled? |
|
|
Typo: flieds. |
|
|
Col: 40 Missing semicolon. |
![]() |
|
Missing ; |
|
|
Col: 64 Missing semicolon. |
![]() |
|
Missing ; |
|
|
Thes should be indented 1 space. No need for />. Same elsewhere. These apply to the other templates as well. |
|
|
F821 undefined name 'ModuleNotFoundError' |
![]() |
|
F401 'django.contrib.auth.login' imported but unused |
![]() |
|
F401 'reviewboard.accounts.backends.standard.StandardAuthBackend' imported but unused |
![]() |
|
F401 'django.utils.translation.gettext_lazy as _' imported but unused |
![]() |
|
"Single Sign-On" is the more "official" way it's written. Let's use that everywhere. |
|
|
We use "Identity Provider", "IdP", and "identity provider" in the document. We should stick with one term and capitalization. We … |
|
|
What do you think about breaking this down in bullet point form, like: You Identity Provider should provide the following: … |
|
|
We should probably make it clear that these are examples and make sure they don't link: Example: ``https://.../` Worth describing … |
|
|
Trying to start moving our initial signal setup into the AppConfig and signal_handlers.py, so we can avoid side-effects on module … |
|
|
I have a couple of additional thoughts regarding this model. These are pure discussion topics. I don't have concrete answers. … |
|
|
I hope 64 is fine, but I wonder if we might want to give some more breathing room. Maybe 128? |
|
|
We could probably put that in the help text. |
|
|
If we do add a LocalSite relation, we'll need that in here. However, I'm not sure that we want to … |
|
|
Let's standardize on "Single Sign-On". |
|
|
We can probably just make this a urls = [] attribute by default, and let subclasses decide whether to use … |
|
|
Can we break this down into a numbered list of items, for readability? This should also document what the second … |
|
|
Since this is a new registry, we can probably avoid entrypoints altogether, just require extensions to register items. |
|
|
If we don't need to support entrypoints, can we remove this? |
|
|
We can probably remove this blank line. |
|
|
These can probably begin on the include( line. |
|
|
I wonder if we could find a better place for this, because there are other places where we're going to … |
|
|
This label should be in sentence casing. |
|
|
This label should be in sentence casing (unless "Issuer" is generally capitalized here). |
|
|
These labels should also be in sentence casing. |
|
|
"Identity Provider"? |
|
|
Do we want to remove these? |
|
|
Each of these could start on the path( line. |
|
|
Same notes as on the base implementation. |
|
|
We may want to say "... and restart the web server" |
|
|
Should this be @cached_property? |
|
|
Should this be @cached_property? |
|
|
No need for str() here (unless we're testing via spies an want to ease checking this argument's value). |
|
|
Is this related to the TODO above? Should we drop this, document this? |
|
|
Can we use :term: for things like IdP, and define in glossary.rst? |
|
|
Worth starting to use Enum for these things? (Maybe not -- just throwing this out there.) |
|
|
This will need a Raises: |
|
|
Can we put quotes around the values? |
|
|
Returns goes before Raises. |
|
|
Can we provide the username in question as an argument to this? |
|
|
Can you wrap find_suggested_username in :py:func:? |
|
|
I think I wrote this initially, but it might make sense to limit this to one database query: q = … |
|
|
Swap these. |
|
|
For readability, can we move the name= to the next line? This all kind of blurs together. |
|
|
This should go above the previous sso imports. |
|
|
No trailing period (here and elsewhere). |
|
|
To reduce the amount of code and repeated attribute lookups, can we pull out siteconfig into a local variable? |
|
|
To just reduce database hits during tests, let's just fetch all linked accounts as list(...), and then we can check … |
|
|
Version Added? |
|
|
We do this whole fetch of everything leading up to 'fields' up to twice per forloop iteration. Let's pull this … |
|
|
How was load() being called before? Are we at risk of calling it twice now? |
|
|
We do so many of these. Wonder if it'd make sense to have a private function that yields sso_backend_id, form, … |
|
|
Leftover debugging. |
|
|
Can we remove the blank line? |
|
|
No need for /> anymore. |
|
|
Can we remove the blank line? |
|
|
Can we remove the blank line? |
|
|
No need for the />. |
|
|
Can we remove the blank line? |
|
|
Can we remove the blank line? |
|
|
No need for the /> |
|
|
Can we remove the blank line? |
|
|
F401 'django_evolution.mutations.AddField' imported but unused |
![]() |
|
E402 module level import not at top of file |
![]() |
|
Can this be super()? |
|
|
Since we've eliminated the entrypoint support, we no longer need this. |
|
|
Can this be super()? |
|
|
Can this be super()? |
|
|
Can this be super()? |
|
|
Can this be super()? |
|
|
Can this be super()? |
|
|
We should probably get that into Asana for tracking. What's involved here? Can we address this for 5.0? |
|
|
Can this be super()? |
|
|
Can this be super()? |
|
|
Can this be super()? |
|
|
'djblets.registries.registry.LOAD_ENTRY_POINT' imported but unused Column: 1 Error code: F401 |
![]() |
|
'djblets.db.query.get_object_or_none' imported but unused Column: 1 Error code: F401 |
![]() |
reviewboard/accounts/sso/backends/saml/sso_backend.py (Diff revision 1) |
---|
I think this all looks really good. There are a lot of moving pieces, so I'm not sure how much I've internalized (it would be nice to ultimately break this out into a change for the subform code, and then maybe SSO vs. SAML implementation), but nothing stands out as wrong.
I have a handful of comments in here. Most are things like doc fixes or style/ordering fixes, which I stopped providing because I know it's WIP. Only a few are on design.
reviewboard/accounts/__init__.py (Diff revision 1) |
---|
Do we need to do this, or can we rely on lazy population in the registry?
reviewboard/accounts/models.py (Diff revision 1) |
---|
extra_data
is worth having, but I think we may want to include something specific for credential data that we'd never want to expose via the API.HostingServiceAccount
has adata = JSONField()
where those go. Maybe aservice_data =
would be appropriate.
reviewboard/accounts/sso/backends/registry.py (Diff revision 1) |
---|
Rather than this format, cna we do:
for _module, _backend_cls_name in (('saml.sso_backend', 'SAMLSSOBackend'),): ...
Or define the list separately.
reviewboard/accounts/sso/backends/saml/sso_backend.py (Diff revision 1) |
---|
This should be after
__init__
.
reviewboard/accounts/sso/backends/saml/views.py (Diff revision 1) |
---|
We should have some
else
case, even if that's just raising an exception.
reviewboard/accounts/sso/backends/saml/views.py (Diff revision 1) |
---|
No need to
.save()
after.create()
.
reviewboard/accounts/sso/backends/saml/views.py (Diff revision 1) |
---|
Shouldn't have a trailing comma for a function call.
reviewboard/accounts/sso/users.py (Diff revision 1) |
---|
Missing a
Version Added
. We should put them in the functions as well.Same comments for other new modules.
reviewboard/admin/forms/auth_settings.py (Diff revision 1) |
---|
Let's wrap any calls in an exception, in case any extension-provided backends (or ours) ever misbehave.
reviewboard/admin/forms/auth_settings.py (Diff revision 1) |
---|
Should we conditionally save ones with
_enabled
?
reviewboard/templates/accounts/sso/link-user-connect-existing.html (Diff revision 1) |
---|
Thes should be indented 1 space.
No need for
/>
. Same elsewhere.These apply to the other templates as well.
Description: |
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
Testing Done: |
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
Commits: |
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
Diff: |
Revision 2 (+3742 -42) |
reviewboard/accounts/sso/backends/saml/sso_backend.py (Diff revision 2) |
---|
F821 undefined name 'ModuleNotFoundError'
Switch to
ImportError
for import test, since that's a parent class ofModuleNotFoundError
, and works with python 2.7 and the version of flake8 we're running.
Commits: |
|
|||||||||
---|---|---|---|---|---|---|---|---|---|---|
Diff: |
Revision 3 (+3742 -42) |
login_user
method.Description: |
|
|||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
Commits: |
|
|||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
Diff: |
Revision 4 (+3890 -42) |
reviewboard/accounts/sso/backends/saml/views.py (Diff revision 4) |
---|
F401 'django.contrib.auth.login' imported but unused
reviewboard/accounts/sso/backends/saml/views.py (Diff revision 4) |
---|
F401 'reviewboard.accounts.backends.standard.StandardAuthBackend' imported but unused
Commits: |
|
|||||||||
---|---|---|---|---|---|---|---|---|---|---|
Diff: |
Revision 5 (+3886 -42) |
Description: |
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
Commits: |
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
Diff: |
Revision 6 (+4432 -46) |
Fix up description/testing done.
Description: |
|
|||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
Testing Done: |
|
Remove debug print statements.
Commits: |
|
|||||||||
---|---|---|---|---|---|---|---|---|---|---|
Diff: |
Revision 7 (+4420 -46) |
Out of WIP.
Summary: |
|
|||||||||||||||||||||||||||||||||||||||||||||||||||||||||
---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
Description: |
|
|||||||||||||||||||||||||||||||||||||||||||||||||||||||||
Commits: |
|
|||||||||||||||||||||||||||||||||||||||||||||||||||||||||
Diff: |
Revision 8 (+4838 -46) |
reviewboard/accounts/sso/backends/saml/views.py (Diff revision 8) |
---|
F401 'django.utils.translation.gettext_lazy as _' imported but unused
Commits: |
|
|||||||||
---|---|---|---|---|---|---|---|---|---|---|
Diff: |
Revision 9 (+4836 -46) |
This is great. Thanks again for taking this on!
Lots of comments in here, since it's a large. Mostly style, some doc suggestions, food-for-thought, and some things you're more than welcome to defer (future code improvement stuff). There is one larger topic about
LinkedAccount
model fields that I think we'll want to figure out.
docs/manual/admin/configuration/authentication-settings.rst (Diff revision 9) |
---|
"Single Sign-On" is the more "official" way it's written. Let's use that everywhere.
docs/manual/admin/configuration/authentication-settings.rst (Diff revision 9) |
---|
We use "Identity Provider", "IdP", and "identity provider" in the document. We should stick with one term and capitalization.
We could wrap each in
:term:
and then define it more closely inglossary.rst
, if that's worth it. That at least would also allow us to group the variations (Identity Provider
,IdP
).
docs/manual/admin/configuration/authentication-settings.rst (Diff revision 9) |
---|
What do you think about breaking this down in bullet point form, like:
You Identity Provider should provide the following: 1. URLs to fill out for the Issuer and SAML/SLO endpoints. 2. A copy of your X.509 certificate. 3. ...
docs/manual/admin/configuration/authentication-settings.rst (Diff revision 9) |
---|
We should probably make it clear that these are examples and make sure they don't link:
Example: ``https://.../`
Worth describing these at all?
reviewboard/accounts/__init__.py (Diff revision 9) |
---|
Trying to start moving our initial signal setup into the
AppConfig
andsignal_handlers.py
, so we can avoid side-effects on module import. I just did this with /r/12299/, for reference. Can we adopt that pattern here?
reviewboard/accounts/models.py (Diff revision 9) |
---|
I have a couple of additional thoughts regarding this model.
These are pure discussion topics. I don't have concrete answers.
Local Sites?
Do we want to bind this to a
LocalSite
?An argument for that would be that if an organization shuts down their, say, RBCommons account, and a user retains their linked account, both we and they retain some link to an account that the organization may have expected to have been deleted.
Having a
LocalSite
relation would mean that a user would have to link up their accounts per-LocalSite
, but that's probably a lot less common and maybe even preferable.Linked account type
I think it'd be really useful to be able to identify the type of a linked account. I know we can query by service, but something more broad can be useful for linked account management.
I don't know what this would look like, or at what level we would want to clarify it.
Maybe we differentiate "auth" from "chat" from "scm" or something.
Or maybe instead of/in addition, we want to bind an integration ID, for any integrations that will use this.
Again, no idea, but for querying/management purposes, I think we'd want something beyond
service_id
.
reviewboard/accounts/models.py (Diff revision 9) |
---|
I hope 64 is fine, but I wonder if we might want to give some more breathing room. Maybe 128?
reviewboard/accounts/models.py (Diff revision 9) |
---|
If we do add a
LocalSite
relation, we'll need that in here.However, I'm not sure that we want to limit 1 user account per service ID. Instead, do we want this to be
('service_user_id', 'service_id')
?
reviewboard/accounts/sso/backends/__init__.py (Diff revision 9) |
---|
Let's standardize on "Single Sign-On".
reviewboard/accounts/sso/backends/base.py (Diff revision 9) |
---|
We can probably just make this a
urls = []
attribute by default, and let subclasses decide whether to use@property
.
reviewboard/accounts/sso/backends/base.py (Diff revision 9) |
---|
Can we break this down into a numbered list of items, for readability?
This should also document what the second item looks like if the first is
True
.
reviewboard/accounts/sso/backends/registry.py (Diff revision 9) |
---|
Since this is a new registry, we can probably avoid entrypoints altogether, just require extensions to register items.
reviewboard/accounts/sso/backends/registry.py (Diff revision 9) |
---|
If we don't need to support entrypoints, can we remove this?
reviewboard/accounts/sso/backends/registry.py (Diff revision 9) |
---|
We can probably remove this blank line.
reviewboard/accounts/sso/backends/registry.py (Diff revision 9) |
---|
These can probably begin on the
include(
line.
reviewboard/accounts/sso/backends/saml/forms.py (Diff revision 9) |
---|
I wonder if we could find a better place for this, because there are other places where we're going to want X.509 certs to be validated (such as self-signed certs for repos/LDAP/etc. eventually).
We probably should have a
reviewboard.crypto
module, or even adjblets.crypto
. We currently have some crypto utils inreviewboard.scmtools.crypto_utils
, and we could put this in there for now, but it might be a good time to think about a better location.This does not have to be part of this change.
reviewboard/accounts/sso/backends/saml/forms.py (Diff revision 9) |
---|
This label should be in sentence casing.
reviewboard/accounts/sso/backends/saml/forms.py (Diff revision 9) |
---|
This label should be in sentence casing (unless "Issuer" is generally capitalized here).
reviewboard/accounts/sso/backends/saml/forms.py (Diff revision 9) |
---|
These labels should also be in sentence casing.
reviewboard/accounts/sso/backends/saml/sso_backend.py (Diff revision 9) |
---|
Each of these could start on the
path(
line.
reviewboard/accounts/sso/backends/saml/sso_backend.py (Diff revision 9) |
---|
Same notes as on the base implementation.
reviewboard/accounts/sso/backends/saml/sso_backend.py (Diff revision 9) |
---|
We may want to say "... and restart the web server"
reviewboard/accounts/sso/backends/saml/views.py (Diff revision 9) |
---|
No need for
str()
here (unless we're testing via spies an want to ease checking this argument's value).
reviewboard/accounts/sso/backends/saml/views.py (Diff revision 9) |
---|
Is this related to the TODO above? Should we drop this, document this?
reviewboard/accounts/sso/backends/saml/views.py (Diff revision 9) |
---|
Can we use
:term:
for things likeIdP
, and define inglossary.rst
?
reviewboard/accounts/sso/backends/saml/views.py (Diff revision 9) |
---|
Worth starting to use
Enum
for these things? (Maybe not -- just throwing this out there.)
reviewboard/accounts/sso/backends/saml/views.py (Diff revision 9) |
---|
Can we put quotes around the values?
reviewboard/accounts/sso/users.py (Diff revision 9) |
---|
Can we provide the username in question as an argument to this?
reviewboard/accounts/sso/users.py (Diff revision 9) |
---|
Can you wrap
find_suggested_username
in:py:func:
?
reviewboard/accounts/sso/users.py (Diff revision 9) |
---|
I think I wrote this initially, but it might make sense to limit this to one database query:
q = Q() if username: q |= Q(username=username) q |= Q(email=email) | Q(username=alternate_username) return get_object_or_none(User, q)
Actually though, whether we do this or not, we can't rely on
get_object_or_none()
alone when querying
reviewboard/accounts/sso/views.py (Diff revision 9) |
---|
For readability, can we move the
name=
to the next line? This all kind of blurs together.
reviewboard/accounts/tests/test_saml_forms.py (Diff revision 9) |
---|
This should go above the previous
sso
imports.
reviewboard/accounts/tests/test_saml_forms.py (Diff revision 9) |
---|
No trailing period (here and elsewhere).
reviewboard/accounts/tests/test_saml_forms.py (Diff revision 9) |
---|
To reduce the amount of code and repeated attribute lookups, can we pull out
siteconfig
into a local variable?
reviewboard/accounts/tests/test_saml_views.py (Diff revision 9) |
---|
To just reduce database hits during tests, let's just fetch all linked accounts as
list(...)
, and then we can check length and inspect the first result without further queries.Same below.
reviewboard/admin/forms/auth_settings.py (Diff revision 9) |
---|
We do this whole fetch of everything leading up to
'fields'
up to twice per forloop iteration. Let's pull this out before the loop. That'll help it read a bit better, too.
reviewboard/admin/forms/auth_settings.py (Diff revision 9) |
---|
How was
load()
being called before? Are we at risk of calling it twice now?
reviewboard/admin/forms/auth_settings.py (Diff revision 9) |
---|
We do so many of these. Wonder if it'd make sense to have a private function that yields
sso_backend_id, form, enable_field_id
.Just a cleanup suggestion, but not critical.
reviewboard/templates/accounts/sso/link-user-connect-existing.html (Diff revision 9) |
---|
Can we remove the blank line?
reviewboard/templates/accounts/sso/link-user-connect-existing.html (Diff revision 9) |
---|
No need for
/>
anymore.
reviewboard/templates/accounts/sso/link-user-connect-existing.html (Diff revision 9) |
---|
Can we remove the blank line?
reviewboard/templates/accounts/sso/link-user-login.html (Diff revision 9) |
---|
Can we remove the blank line?
reviewboard/templates/accounts/sso/link-user-login.html (Diff revision 9) |
---|
Can we remove the blank line?
reviewboard/templates/accounts/sso/link-user-provision.html (Diff revision 9) |
---|
Can we remove the blank line?
reviewboard/templates/accounts/sso/link-user-provision.html (Diff revision 9) |
---|
Can we remove the blank line?
Commits: |
|
|||||||||
---|---|---|---|---|---|---|---|---|---|---|
Diff: |
Revision 10 (+4898 -52) |
reviewboard/accounts/evolutions/linkedaccount_unique_together.py (Diff revision 10) |
---|
F401 'django_evolution.mutations.AddField' imported but unused
Commits: |
|
|||||||||
---|---|---|---|---|---|---|---|---|---|---|
Diff: |
Revision 11 (+4910 -54) |
Remove unintentional changes
Commits: |
|
|||||||||
---|---|---|---|---|---|---|---|---|---|---|
Diff: |
Revision 12 (+4898 -52) |
Commits: |
|
|||||||||
---|---|---|---|---|---|---|---|---|---|---|
Diff: |
Revision 13 (+4984 -52) |
This looks great. I'm happy with it. I just have a few small optional nits, a thing for tracking a TODO, and then I have a reply to the 3 queries bit in my previous review.
reviewboard/accounts/sso/backends/registry.py (Diff revision 13) |
---|
Since we've eliminated the entrypoint support, we no longer need this.
reviewboard/accounts/sso/backends/saml/views.py (Diff revision 13) |
---|
We should probably get that into Asana for tracking.
What's involved here? Can we address this for 5.0?
Commits: |
|
|||||||||
---|---|---|---|---|---|---|---|---|---|---|
Diff: |
Revision 14 (+4992 -52) |
reviewboard/accounts/sso/backends/registry.py (Diff revision 14) |
---|
'djblets.registries.registry.LOAD_ENTRY_POINT' imported but unused Column: 1 Error code: F401
reviewboard/accounts/sso/users.py (Diff revision 14) |
---|
'djblets.db.query.get_object_or_none' imported but unused Column: 1 Error code: F401
Commits: |
|
|||||||||
---|---|---|---|---|---|---|---|---|---|---|
Diff: |
Revision 15 (+4990 -52) |