[Bug 64442] New: Re-use roles and groups defined on users on MemoryUserDatabase creation

classic Classic list List threaded Threaded
18 messages Options
Reply | Threaded
Open this post in threaded view
|

[Bug 64442] New: Re-use roles and groups defined on users on MemoryUserDatabase creation

Bugzilla from bugzilla@apache.org
https://bz.apache.org/bugzilla/show_bug.cgi?id=64442

            Bug ID: 64442
           Summary: Re-use roles and groups defined on users on
                    MemoryUserDatabase creation
           Product: Tomcat 10
           Version: unspecified
          Hardware: All
                OS: All
            Status: NEW
          Severity: minor
          Priority: P2
         Component: Catalina
          Assignee: [hidden email]
          Reporter: [hidden email]
  Target Milestone: ------

Created attachment 37246
  --> https://bz.apache.org/bugzilla/attachment.cgi?id=37246&action=edit
Re-use roles and groups defined on users on MemoryUserDatabase creation

When the XML file for MemoryUserDatabse is digested, the order of the
elements was important. It had to be roles, groups and than users.
With this patch the order of the elements is not important any more.
If a user element defined a role or group before the corresponding
role or group element, we now will re-use that element and add a
possibly missing description.

--
You are receiving this mail because:
You are the assignee for the bug.
---------------------------------------------------------------------
To unsubscribe, e-mail: [hidden email]
For additional commands, e-mail: [hidden email]

Reply | Threaded
Open this post in threaded view
|

[Bug 64442] Re-use roles and groups defined on users on MemoryUserDatabase creation

Bugzilla from bugzilla@apache.org
https://bz.apache.org/bugzilla/show_bug.cgi?id=64442

--- Comment #1 from Christopher Schultz <[hidden email]> ---
What's the desire, here? Making the XML more readable or avoiding redefining
roles or groups?

If you are going to modify the code, you could also modify the XML Schema to
suit (/conf/tomcat-users.xsd).

--
You are receiving this mail because:
You are the assignee for the bug.
---------------------------------------------------------------------
To unsubscribe, e-mail: [hidden email]
For additional commands, e-mail: [hidden email]

Reply | Threaded
Open this post in threaded view
|

[Bug 64442] Re-use roles and groups defined on users on MemoryUserDatabase creation

Bugzilla from bugzilla@apache.org
In reply to this post by Bugzilla from bugzilla@apache.org
https://bz.apache.org/bugzilla/show_bug.cgi?id=64442

--- Comment #2 from Felix Schumacher <[hidden email]> ---
The trigger here was, that we tried to add a role without a XML-aware editor
and placed the role definition behind the user, that also defined the role. The
result of such a misconfiguration is, a user that is not member of that role.
This is confusing. And the patch tries to be more forgiving with
misconfiguration.

Hadn't thought about adapting the XSD, though. Do you think we should adapt it?

Another possible way to let users know the misconfiguration is actually
checking the conformance of the XML file, but I think the more forgiving
approach is nicer (to the user).

--
You are receiving this mail because:
You are the assignee for the bug.
---------------------------------------------------------------------
To unsubscribe, e-mail: [hidden email]
For additional commands, e-mail: [hidden email]

Reply | Threaded
Open this post in threaded view
|

[Bug 64442] Re-use roles and groups defined on users on MemoryUserDatabase creation

Bugzilla from bugzilla@apache.org
In reply to this post by Bugzilla from bugzilla@apache.org
https://bz.apache.org/bugzilla/show_bug.cgi?id=64442

--- Comment #3 from mgrigorov <[hidden email]> ---
Is the XSD actually being used by Tomcat to validate the .xml when parsing ?
I guess it is not, otherwise Felix would have seen an error in the logs
explaining that the order is not correct.

--
You are receiving this mail because:
You are the assignee for the bug.
---------------------------------------------------------------------
To unsubscribe, e-mail: [hidden email]
For additional commands, e-mail: [hidden email]

Reply | Threaded
Open this post in threaded view
|

[Bug 64442] Re-use roles and groups defined on users on MemoryUserDatabase creation

Bugzilla from bugzilla@apache.org
In reply to this post by Bugzilla from bugzilla@apache.org
https://bz.apache.org/bugzilla/show_bug.cgi?id=64442

--- Comment #4 from Christopher Schultz <[hidden email]> ---
I think if we are going to provide an XSD for the file, our code should match
it. Or the other way around. In any case, they should agree. :)

Tomcat does not actually bother to enforce the schema's rules when parsing the
file. But historically, there was no schema; that's a (relatively) late
addition (2019, I think). Now that it exists, it /could/ be used for validation
during database-loading.

Maybe for Tomcat 10?

--
You are receiving this mail because:
You are the assignee for the bug.
---------------------------------------------------------------------
To unsubscribe, e-mail: [hidden email]
For additional commands, e-mail: [hidden email]

Reply | Threaded
Open this post in threaded view
|

[Bug 64442] Re-use roles and groups defined on users on MemoryUserDatabase creation

Bugzilla from bugzilla@apache.org
In reply to this post by Bugzilla from bugzilla@apache.org
https://bz.apache.org/bugzilla/show_bug.cgi?id=64442

--- Comment #5 from Felix Schumacher <[hidden email]> ---
I think the code should either work without surprises and I was surprised, that
adding a role (even at the wrong place) led to a user without a role, or fail
fast and log a warning.

My preference would be to be forgiving, but if you think we should be more
strict, than I could try to see, if I can enable the loader to enforce the
schema.

--
You are receiving this mail because:
You are the assignee for the bug.
---------------------------------------------------------------------
To unsubscribe, e-mail: [hidden email]
For additional commands, e-mail: [hidden email]

Reply | Threaded
Open this post in threaded view
|

[Bug 64442] Re-use roles and groups defined on users on MemoryUserDatabase creation

Bugzilla from bugzilla@apache.org
In reply to this post by Bugzilla from bugzilla@apache.org
https://bz.apache.org/bugzilla/show_bug.cgi?id=64442

--- Comment #6 from Christopher Schultz <[hidden email]> ---
No, I'm okay with both the relaxed behavior and not requiring the XSD
validation. I just want the XSD to reflect what the code is willing to accept.

--
You are receiving this mail because:
You are the assignee for the bug.
---------------------------------------------------------------------
To unsubscribe, e-mail: [hidden email]
For additional commands, e-mail: [hidden email]

Reply | Threaded
Open this post in threaded view
|

[Bug 64442] Re-use roles and groups defined on users on MemoryUserDatabase creation

Bugzilla from bugzilla@apache.org
In reply to this post by Bugzilla from bugzilla@apache.org
https://bz.apache.org/bugzilla/show_bug.cgi?id=64442

--- Comment #7 from Mark Thomas <[hidden email]> ---
The XSD was added to document the requirements / allow XML aware editors to
produce valid files.

I'm happy with relaxing the restrictions.

With the relaxation in place, we need to think about what duplicate definition
of roles / groups / users means and how to handle it.

--
You are receiving this mail because:
You are the assignee for the bug.
---------------------------------------------------------------------
To unsubscribe, e-mail: [hidden email]
For additional commands, e-mail: [hidden email]

Reply | Threaded
Open this post in threaded view
|

[Bug 64442] Re-use roles and groups defined on users on MemoryUserDatabase creation

Bugzilla from bugzilla@apache.org
In reply to this post by Bugzilla from bugzilla@apache.org
https://bz.apache.org/bugzilla/show_bug.cgi?id=64442

--- Comment #8 from Remy Maucherat <[hidden email]> ---
Did I miss something, or is a random element order much harder to write in a
xsd ? The sequence is very easy otoh, but the ordering is fixed.

I'm ok as well for allowing creativity here, it doesn't hurt. People can fully
respect the schema if they like it better.

--
You are receiving this mail because:
You are the assignee for the bug.
---------------------------------------------------------------------
To unsubscribe, e-mail: [hidden email]
For additional commands, e-mail: [hidden email]

Reply | Threaded
Open this post in threaded view
|

[Bug 64442] Re-use roles and groups defined on users on MemoryUserDatabase creation

Bugzilla from bugzilla@apache.org
In reply to this post by Bugzilla from bugzilla@apache.org
https://bz.apache.org/bugzilla/show_bug.cgi?id=64442

--- Comment #9 from Felix Schumacher <[hidden email]> ---
OK, so I will try to relax the schema.

Currently a role, that is defined after a user has "defined" a role, will reset
the membership and therefore loose the connection to the user. That has been
fixed with the patch. Another problem was, that a role - again defined through
an user entry - had no description. That has been fixed with the patch, too.

Now, the only part undefined would be, when a xml file contained a role (or
user or group) twice (with different descriptions). Then only the first
definition would be used (loosing the second one). But the users would still be
connected to a role with the correct name.

So given a xml structure like

 <user name="u1" roles="r1"/>
 <role name="r1" description="first"/>
 <user name="u2" roles="r1"/>
 <role name="r1" description="second"/>

would lead to one role (r1 with description "first"), that is connected to the
users u1 and u2.

Currently this would lead to two users u1 and u2 that are not connected to any
role and one role (r1 with description "second").

And yes, I know, that the xml structure was not valid according to our xsd :)

--
You are receiving this mail because:
You are the assignee for the bug.
---------------------------------------------------------------------
To unsubscribe, e-mail: [hidden email]
For additional commands, e-mail: [hidden email]

Reply | Threaded
Open this post in threaded view
|

[Bug 64442] Re-use roles and groups defined on users on MemoryUserDatabase creation

Bugzilla from bugzilla@apache.org
In reply to this post by Bugzilla from bugzilla@apache.org
https://bz.apache.org/bugzilla/show_bug.cgi?id=64442

Felix Schumacher <[hidden email]> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
  Attachment #37246|0                           |1
        is obsolete|                            |

--- Comment #10 from Felix Schumacher <[hidden email]> ---
Created attachment 37260
  --> https://bz.apache.org/bugzilla/attachment.cgi?id=37260&action=edit
Re-use roles and groups defined on users on MemoryUserDatabase creation

Re-use roles that may have been created earlier through user entries and relax
the schema to allow such constellations.

--
You are receiving this mail because:
You are the assignee for the bug.
---------------------------------------------------------------------
To unsubscribe, e-mail: [hidden email]
For additional commands, e-mail: [hidden email]

Reply | Threaded
Open this post in threaded view
|

[Bug 64442] Re-use roles and groups defined on users on MemoryUserDatabase creation

Bugzilla from bugzilla@apache.org
In reply to this post by Bugzilla from bugzilla@apache.org
https://bz.apache.org/bugzilla/show_bug.cgi?id=64442

--- Comment #11 from Christopher Schultz <[hidden email]> ---
(In reply to Remy Maucherat from comment #8)
> Did I miss something, or is a random element order much harder to write in a
> xsd ? The sequence is very easy otoh, but the ordering is fixed.

Nope. Felix's latest proposal uses <xs:all> as a wrapper (which I've never used
before), but you could also do this, which I think is clearer:

<xs:choose minOccurs="0" maxOccurs="unbounded">
  // element role
  // element group
  // element user
  ...
</xs:choose>

--
You are receiving this mail because:
You are the assignee for the bug.
---------------------------------------------------------------------
To unsubscribe, e-mail: [hidden email]
For additional commands, e-mail: [hidden email]

Reply | Threaded
Open this post in threaded view
|

[Bug 64442] Re-use roles and groups defined on users on MemoryUserDatabase creation

Bugzilla from bugzilla@apache.org
In reply to this post by Bugzilla from bugzilla@apache.org
https://bz.apache.org/bugzilla/show_bug.cgi?id=64442

--- Comment #12 from Felix Schumacher <[hidden email]> ---
I didn't find anything about xs:choose. Did you mean xs:choice? (That would not
work, as it would allow only one of the three elements).

--
You are receiving this mail because:
You are the assignee for the bug.
---------------------------------------------------------------------
To unsubscribe, e-mail: [hidden email]
For additional commands, e-mail: [hidden email]

Reply | Threaded
Open this post in threaded view
|

[Bug 64442] Re-use roles and groups defined on users on MemoryUserDatabase creation

Bugzilla from bugzilla@apache.org
In reply to this post by Bugzilla from bugzilla@apache.org
https://bz.apache.org/bugzilla/show_bug.cgi?id=64442

--- Comment #13 from Mark Thomas <[hidden email]> ---
The way the servlet XSD does things is with a nested choice.

--
You are receiving this mail because:
You are the assignee for the bug.
---------------------------------------------------------------------
To unsubscribe, e-mail: [hidden email]
For additional commands, e-mail: [hidden email]

Reply | Threaded
Open this post in threaded view
|

[Bug 64442] Re-use roles and groups defined on users on MemoryUserDatabase creation

Bugzilla from bugzilla@apache.org
In reply to this post by Bugzilla from bugzilla@apache.org
https://bz.apache.org/bugzilla/show_bug.cgi?id=64442

--- Comment #14 from Felix Schumacher <[hidden email]> ---
Yes, you can use a choice-element, if it is used together with the attributes
minOccurs and maxOccurs, but I fail to see, how it is clearer than using the
all-element.

But as it is clearer for Chris, it is used in the servlet xsd and I used all
only because I found it first while researching a replacement, I will use
choice :)

--
You are receiving this mail because:
You are the assignee for the bug.
---------------------------------------------------------------------
To unsubscribe, e-mail: [hidden email]
For additional commands, e-mail: [hidden email]

Reply | Threaded
Open this post in threaded view
|

[Bug 64442] Re-use roles and groups defined on users on MemoryUserDatabase creation

Bugzilla from bugzilla@apache.org
In reply to this post by Bugzilla from bugzilla@apache.org
https://bz.apache.org/bugzilla/show_bug.cgi?id=64442

--- Comment #15 from Christopher Schultz <[hidden email]> ---
(In reply to Felix Schumacher from comment #12)
> I didn't find anything about xs:choose. Did you mean xs:choice? (That would
> not work, as it would allow only one of the three elements).

Yes, I meant "choice". The unbounded choice allows any number of any of the
items contained within he choice (not just any number of whichever one you
choose).

--
You are receiving this mail because:
You are the assignee for the bug.
---------------------------------------------------------------------
To unsubscribe, e-mail: [hidden email]
For additional commands, e-mail: [hidden email]

Reply | Threaded
Open this post in threaded view
|

[Bug 64442] Re-use roles and groups defined on users on MemoryUserDatabase creation

Bugzilla from bugzilla@apache.org
In reply to this post by Bugzilla from bugzilla@apache.org
https://bz.apache.org/bugzilla/show_bug.cgi?id=64442

--- Comment #16 from Mark Thomas <[hidden email]> ---
Do you want to commit this change before I tag the next release next week?

--
You are receiving this mail because:
You are the assignee for the bug.
---------------------------------------------------------------------
To unsubscribe, e-mail: [hidden email]
For additional commands, e-mail: [hidden email]

Reply | Threaded
Open this post in threaded view
|

[Bug 64442] Re-use roles and groups defined on users on MemoryUserDatabase creation

Bugzilla from bugzilla@apache.org
In reply to this post by Bugzilla from bugzilla@apache.org
https://bz.apache.org/bugzilla/show_bug.cgi?id=64442

--- Comment #17 from Felix Schumacher <[hidden email]> ---
I commited it to trunk and to the 9.0.x tree. Should I backport it to 8.5.x,
too?

--
You are receiving this mail because:
You are the assignee for the bug.
---------------------------------------------------------------------
To unsubscribe, e-mail: [hidden email]
For additional commands, e-mail: [hidden email]