Party Manager Permissions Issues

[Adrian Crum]

I'm working on moving the embedded party services permission checking to the new permission
services. I have run into a few issues I believe need discussion.

1. The current createPerson service doesn't require a logged-in user or any permission to run. I
understand this is so a new user can create an account for themselves, but it seems to leave a
security gap in the project. A malicious user could create thousands of phony persons.

I'll leave it as-is for now. Perhaps a better approach would be to have the service perform a
permission check by default, and have specific events override the default permission check with a
custom permission check - to allow new users to create an account for themselves. We can tackle that
in the future.

2. The current general PARTYMGR CRUD permissions have extensions for particular tasks, like
PARTYMGR_STS_xxx for party status operations. The current embedded permission checking logic checks
for PARTYMGR_STS_xxx permissions, but not the base PARTYMGR permissions. I'm not sure if this is the
way it should work. For example, the PARTYMGR_CREATE permission description says "Create operations
in the party manager" - which should include creating a party status. The current logic doesn't
allow that.

I propose checking the current extended permissions in the following way: Check for the base
PARTYMGR permission first, if that fails, then check the extended (PARTYMGR_STS) permission.
Example: the setPartyStatus service would check for the PARTYMGR_UPDATE permission first, and if
that fails, it would then check for the PARTYMGR_STS_UPDATE permission.

I used PARTYMGR_STS as the example here, but the same holds true for the other extended permissions:
PARTYMGR_CME, PARTYMGR_GRP, PARTYMGR_PCM, PARTYMGR_QAL, PARTYMGR_REL, PARTYMGR_ROLE, and PARTYMGR_SRC.

What do you think?

-Adrian

[Jacopo Cappellato]

Adrian Crum wrote:

> ...
> I propose checking the current extended permissions in the following
> way: Check for the base PARTYMGR permission first, if that fails, then
> check the extended (PARTYMGR_STS) permission. Example: the
> setPartyStatus service would check for the PARTYMGR_UPDATE permission
> first, and if that fails, it would then check for the
> PARTYMGR_STS_UPDATE permission.
> ...
> What do you think?
>

Looks good to me.

Jacopo

[Jacopo Cappellato]

Jacopo Cappellato wrote:

> Adrian Crum wrote:
>> ...
>> I propose checking the current extended permissions in the following
>> way: Check for the base PARTYMGR permission first, if that fails, then
>> check the extended (PARTYMGR_STS) permission. Example: the
>> setPartyStatus service would check for the PARTYMGR_UPDATE permission
>> first, and if that fails, it would then check for the
>> PARTYMGR_STS_UPDATE permission.
>> ...
>> What do you think?
>>

Adrian, are you going to work also on the ROLE based permissions checks?
They are trickier but it would be great to refactor them too.

Jacopo

[Adrian Crum]

Jacopo,

Right now I'm doing a simple move. I'm moving the permissions checking logic found in
PartyServices.java to the permission service simple methods. I wasn't planning on refactoring
anything. So, if the ROLE based permission checks are already in PartyServices.java, then yes, I
will be working on them.

The only reason I brought up the extended permissions issue was because I was going to make the move
without changing anything, but while doing so I noticed the existing permissions check didn't have
the desired result.

-Adrian

Jacopo Cappellato wrote:

> Jacopo Cappellato wrote:
>
>> Adrian Crum wrote:
>>
>>> ...
>>> I propose checking the current extended permissions in the following
>>> way: Check for the base PARTYMGR permission first, if that fails,
>>> then check the extended (PARTYMGR_STS) permission. Example: the
>>> setPartyStatus service would check for the PARTYMGR_UPDATE permission
>>> first, and if that fails, it would then check for the
>>> PARTYMGR_STS_UPDATE permission.
>>> ...
>>> What do you think?
>>>
>
> Adrian, are you going to work also on the ROLE based permissions checks?
> They are trickier but it would be great to refactor them too.
>
> Jacopo
>