You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@guacamole.apache.org by Mike Jumper <mi...@guac-dev.org> on 2018/04/03 17:45:20 UTC
User group schema (sanity check)
Hi all,
I'm currently mid-refactor [1] as I propagate the schema changes for user
groups [2] through the database auth. Before I get too far with this, I
wanted to run the current changes by everyone to make sure things are sane.
Overall, user groups are represented by the usual set of tables that each
object has within the database schema:
- guacamole_user_group
- guacamole_user_group_permission
- guacamole_user_group_attribute
In addition, to link a user group with its set of member users/groups, we
have:
- guacamole_user_group_member
As permissions can now be granted to both users and groups, rather than
double the number of permissions tables, I chose to create a base table
common to both users and groups:
- guacamole_entity
This table contains simply an integer primary key, a type (either 'USER' or
'USER_GROUP'), and a name (which must be unique across all entities with
the same type). The guacamole_user and guacamole_user_group tables have a
foreign key into this table, and thus other relations can either be
type-specific (require strictly a user or a group), or can point to both
(this is also why we can have a single "guacamole_user_group_member" table
and not one for user members and another for group members).
For most queries, this makes little difference, however:
- Creating a user (or a group) will require two inserts: one for the
entity, and another for the user/group
- Deleting a user/group will involve deleting the entity (and cascading the
delete to the user/group)
- Retrieving the name of a user/group will always involve a join against
the entity table
The WIP schema changes themselves (I'm working strictly with Postgres until
everything is set in stone):
https://github.com/mike-jumper/guacamole-client/commit/d9afe8a3370ea13228cb8fe001b072648526f66d
Seem sane so far?
- Mike
[1] https://media.giphy.com/media/Mdct4RNEkGIrm/giphy.gif
[2] https://issues.apache.org/jira/browse/GUACAMOLE-220
Re: User group schema (sanity check)
Posted by Nick Couchman <vn...@apache.org>.
>
> >
> Looking closer at LDAP's current handling of groups, I think the necessary
> changes may actually be pretty minimal. I'm in favor of including this in
> scope.
>
>
Sounds good t ome.
> I'll be opening up the first of several PRs shortly with the base API
> changes. The overall set of changes so far is around 8K lines, but I think
> it should be reviewable if broken into logical pieces.
>
>
Wow, that's a lot of changes :-). I'll try to be ready to do some reviews,
but I'll also ask the other committers to be ready, as well - with that
much changing it could probably use a couple of pairs of eyes on it.
> I've also encountered a couple of separate issues which needed to be
> addressed for the sake of groups, and for which I will be opening new JIRA
> issues:
>
> * The UserContext and AuthenticationProvider interfaces continue to be
> cumbersome when applying new API changes, due to the sheer number of
> duplicate stub functions that need to be added to all extensions. I've
> created a AbstractUserContext and AbstractAuthenticationProvider to deal
> with this.
>
>
This actually reminds me of a related topic I've been meaning to bring up
about extensions vs. authentication providers and if we need to maybe
abstract or renaming "Authentication Providers" to just "Guacamole
Extensions." But that's worth a separate thread...
> * The extension system currently properly isolates extensions from each
> other, but does not fully isolate extensions from the webapp. While
> extensions *should* inherit classes from the webapp, any classes included
> with the extension should take priority. I have ClassLoader-related changes
> to the extension system which address that issue.
>
Agreed, and sounds good.
-Nick
Re: User group schema (sanity check)
Posted by Mike Jumper <mi...@guac-dev.org>.
On Wed, Apr 4, 2018 at 10:08 AM, Nick Couchman <vn...@apache.org> wrote:
> ...
> >
> > Good question. The LDAP changes are definitely in scope, and it was my
> > original intent to try to tackle those. Assuming these changes are
> > part of 1.0.0, I can understand limiting the scope to just the
> > database and extension API in the interest of getting the release out.
> > If the group support is part of 1.0.0, I suppose we'll have to see how
> > we feel about release timing vs. including LDAP group support once the
> > database+API portion is complete.
> >
>
> I think it would be preferable to include those changes to the LDAP module
> in 1.0.0, just wasn't sure what the thought was for timing, level of
> effort, etc. Obviously we don't have any particular pressure to release
> 1.0.0 by a certain date, so I don't want to unnecessarily create that
> pressure. I think there will probably be more frustration if we release
> 1.0.0 with support for groups only in the JDBC/API portion and don't do
> LDAP, then there will to wait on the 1.0.0 release a little longer to allow
> for the LDAP changes. But, it's worth discussion - anyone else have any
> thoughts on that?
>
>
Looking closer at LDAP's current handling of groups, I think the necessary
changes may actually be pretty minimal. I'm in favor of including this in
scope.
I'll be opening up the first of several PRs shortly with the base API
changes. The overall set of changes so far is around 8K lines, but I think
it should be reviewable if broken into logical pieces.
I've also encountered a couple of separate issues which needed to be
addressed for the sake of groups, and for which I will be opening new JIRA
issues:
* The UserContext and AuthenticationProvider interfaces continue to be
cumbersome when applying new API changes, due to the sheer number of
duplicate stub functions that need to be added to all extensions. I've
created a AbstractUserContext and AbstractAuthenticationProvider to deal
with this.
* The extension system currently properly isolates extensions from each
other, but does not fully isolate extensions from the webapp. While
extensions *should* inherit classes from the webapp, any classes included
with the extension should take priority. I have ClassLoader-related changes
to the extension system which address that issue.
- Mike
Re: User group schema (sanity check)
Posted by Nick Couchman <vn...@apache.org>.
On Wed, Apr 4, 2018 at 9:27 AM, Mike Jumper <mi...@guac-dev.org>
wrote:
> On Wed, Apr 4, 2018 at 5:40 AM, Nick Couchman <vn...@apache.org> wrote:
> >
> > The schema itself sounds reasonable, my one concern is in the upgrade
> > process for a database that has existing data in it and that needs to
> move
> > from one schema version to the next. This is going to require inserting
> > the existing guacamole_user objects into the guacamole_entity table and
> > then adding the foreign key constraint to the guacamole_user table. Have
> > you done much in the way of testing that upgrade process?
> >
>
> Some. You're definitely right about the upgrade process. The SQL in
> question for the Postgres case is here:
>
> https://github.com/mike-jumper/guacamole-client/blob/
> 3b2ccfe357ba54191732dfd05306f96ea20f7a88/extensions/
> guacamole-auth-jdbc/modules/guacamole-auth-jdbc-postgresql/schema/upgrade/
> upgrade-pre-1.0.0.sql#L126-L164
>
> Other than test things more extensively, and continue to recommend
> that admins back up their databases prior to attempting the upgrade so
> they can rollback if needed, I'm not sure there's any alternative.
>
>
Looks good, just wanted to make sure the upgrade path would be roughly as
simple as it is in the previous schema changes, but it looks fine to me.
> > Also, aside from just the back-end logistics of how to redo the schema
> > while maintaining existing data, are there any gotchas that people will
> > need to be aware of once that schema change takes place in how that is
> > shown on the front-end? That is, if I'm running 0.9.14, and upgrade to
> > 1.0.0 (assuming this is in 1.0.0), and I apply the schema update, update
> my
> > client and server, etc., will I be able to continue to use all of the
> > existing stuff configured in Guacamole prior to the upgrade without
> making
> > additional changes in the admin UI (e.g. re-assigning permissions, etc.)?
> >
>
> Nope. Everything is exactly identical except you can now maintain
> permissions by creating and assigning groups. You can still continue
> to assign permissions on a per-user basis, and existing per-user
> permissions will work as before.
>
Very nice.
>
> > Other than making sure the upgrade process is sorted out, it sounds like
> a
> > great approach to me!
> >
>
> Thanks!
>
> > Oh, one other thing, presumably after the JDBC module changes are done we
> > will have to go back to some of the other modules (LDAP, primarily, I
> would
> > guess) and make some changes, there, correct? Are those in scope of what
> > you're doing here for this particular JIRA issue, or would you consider
> > adding group support to the LDAP module a separate issue?
> >
>
> Good question. The LDAP changes are definitely in scope, and it was my
> original intent to try to tackle those. Assuming these changes are
> part of 1.0.0, I can understand limiting the scope to just the
> database and extension API in the interest of getting the release out.
> If the group support is part of 1.0.0, I suppose we'll have to see how
> we feel about release timing vs. including LDAP group support once the
> database+API portion is complete.
>
I think it would be preferable to include those changes to the LDAP module
in 1.0.0, just wasn't sure what the thought was for timing, level of
effort, etc. Obviously we don't have any particular pressure to release
1.0.0 by a certain date, so I don't want to unnecessarily create that
pressure. I think there will probably be more frustration if we release
1.0.0 with support for groups only in the JDBC/API portion and don't do
LDAP, then there will to wait on the 1.0.0 release a little longer to allow
for the LDAP changes. But, it's worth discussion - anyone else have any
thoughts on that?
>
> Being able to apply the permissions related to group memberships
> defined in another extension (ie: an LDAP user is within an LDAP
> group, but only the group has a corresponding entry+permissions within
> the database) is a critical aspect of the API changes involved, so
> this angle will need to be tested even if LDAP itself is not in scope.
>
>
Probably another reason to just keep LDAP in-scope, but, again, open for
discussion.
-Nick
Re: User group schema (sanity check)
Posted by Mike Jumper <mi...@guac-dev.org>.
On Wed, Apr 4, 2018 at 5:40 AM, Nick Couchman <vn...@apache.org> wrote:
>
> The schema itself sounds reasonable, my one concern is in the upgrade
> process for a database that has existing data in it and that needs to move
> from one schema version to the next. This is going to require inserting
> the existing guacamole_user objects into the guacamole_entity table and
> then adding the foreign key constraint to the guacamole_user table. Have
> you done much in the way of testing that upgrade process?
>
Some. You're definitely right about the upgrade process. The SQL in
question for the Postgres case is here:
https://github.com/mike-jumper/guacamole-client/blob/3b2ccfe357ba54191732dfd05306f96ea20f7a88/extensions/guacamole-auth-jdbc/modules/guacamole-auth-jdbc-postgresql/schema/upgrade/upgrade-pre-1.0.0.sql#L126-L164
Other than test things more extensively, and continue to recommend
that admins back up their databases prior to attempting the upgrade so
they can rollback if needed, I'm not sure there's any alternative.
> Also, aside from just the back-end logistics of how to redo the schema
> while maintaining existing data, are there any gotchas that people will
> need to be aware of once that schema change takes place in how that is
> shown on the front-end? That is, if I'm running 0.9.14, and upgrade to
> 1.0.0 (assuming this is in 1.0.0), and I apply the schema update, update my
> client and server, etc., will I be able to continue to use all of the
> existing stuff configured in Guacamole prior to the upgrade without making
> additional changes in the admin UI (e.g. re-assigning permissions, etc.)?
>
Nope. Everything is exactly identical except you can now maintain
permissions by creating and assigning groups. You can still continue
to assign permissions on a per-user basis, and existing per-user
permissions will work as before.
> Other than making sure the upgrade process is sorted out, it sounds like a
> great approach to me!
>
Thanks!
> Oh, one other thing, presumably after the JDBC module changes are done we
> will have to go back to some of the other modules (LDAP, primarily, I would
> guess) and make some changes, there, correct? Are those in scope of what
> you're doing here for this particular JIRA issue, or would you consider
> adding group support to the LDAP module a separate issue?
>
Good question. The LDAP changes are definitely in scope, and it was my
original intent to try to tackle those. Assuming these changes are
part of 1.0.0, I can understand limiting the scope to just the
database and extension API in the interest of getting the release out.
If the group support is part of 1.0.0, I suppose we'll have to see how
we feel about release timing vs. including LDAP group support once the
database+API portion is complete.
Being able to apply the permissions related to group memberships
defined in another extension (ie: an LDAP user is within an LDAP
group, but only the group has a corresponding entry+permissions within
the database) is a critical aspect of the API changes involved, so
this angle will need to be tested even if LDAP itself is not in scope.
- Mike
Re: User group schema (sanity check)
Posted by Nick Couchman <vn...@apache.org>.
On Tue, Apr 3, 2018 at 1:45 PM, Mike Jumper <mi...@guac-dev.org>
wrote:
> Hi all,
>
> I'm currently mid-refactor [1] as I propagate the schema changes for user
> groups [2] through the database auth. Before I get too far with this, I
> wanted to run the current changes by everyone to make sure things are sane.
>
> Overall, user groups are represented by the usual set of tables that each
> object has within the database schema:
>
> - guacamole_user_group
> - guacamole_user_group_permission
> - guacamole_user_group_attribute
>
> In addition, to link a user group with its set of member users/groups, we
> have:
>
> - guacamole_user_group_member
>
> As permissions can now be granted to both users and groups, rather than
> double the number of permissions tables, I chose to create a base table
> common to both users and groups:
>
> - guacamole_entity
>
> This table contains simply an integer primary key, a type (either 'USER' or
> 'USER_GROUP'), and a name (which must be unique across all entities with
> the same type). The guacamole_user and guacamole_user_group tables have a
> foreign key into this table, and thus other relations can either be
> type-specific (require strictly a user or a group), or can point to both
> (this is also why we can have a single "guacamole_user_group_member" table
> and not one for user members and another for group members).
>
> For most queries, this makes little difference, however:
>
> - Creating a user (or a group) will require two inserts: one for the
> entity, and another for the user/group
> - Deleting a user/group will involve deleting the entity (and cascading the
> delete to the user/group)
> - Retrieving the name of a user/group will always involve a join against
> the entity table
>
> The WIP schema changes themselves (I'm working strictly with Postgres until
> everything is set in stone):
>
> https://github.com/mike-jumper/guacamole-client/commit/
> d9afe8a3370ea13228cb8fe001b072648526f66d
>
> Seem sane so far?
>
>
The schema itself sounds reasonable, my one concern is in the upgrade
process for a database that has existing data in it and that needs to move
from one schema version to the next. This is going to require inserting
the existing guacamole_user objects into the guacamole_entity table and
then adding the foreign key constraint to the guacamole_user table. Have
you done much in the way of testing that upgrade process?
Also, aside from just the back-end logistics of how to redo the schema
while maintaining existing data, are there any gotchas that people will
need to be aware of once that schema change takes place in how that is
shown on the front-end? That is, if I'm running 0.9.14, and upgrade to
1.0.0 (assuming this is in 1.0.0), and I apply the schema update, update my
client and server, etc., will I be able to continue to use all of the
existing stuff configured in Guacamole prior to the upgrade without making
additional changes in the admin UI (e.g. re-assigning permissions, etc.)?
Other than making sure the upgrade process is sorted out, it sounds like a
great approach to me!
Oh, one other thing, presumably after the JDBC module changes are done we
will have to go back to some of the other modules (LDAP, primarily, I would
guess) and make some changes, there, correct? Are those in scope of what
you're doing here for this particular JIRA issue, or would you consider
adding group support to the LDAP module a separate issue?
-Nick