smolnar82 commented on code in PR #1454:
URL: https://github.com/apache/knox/pull/1454#discussion_r4323143878
##########
gateway-server/src/main/java/org/apache/knox/gateway/services/ldap/backend/LdapProxyBackend.java:
##########
@@ -261,9 +269,13 @@ public boolean isSupportedSearchBase(String searchBase) {
if (searchBase == null) {
return false;
}
- String searchBaseLowerCase = searchBase.toLowerCase(ROOT);
- return searchBaseLowerCase.endsWith(proxyBaseDn.toLowerCase(ROOT)) ||
- searchBaseLowerCase.endsWith(remoteBaseDn.toLowerCase(ROOT));
+ try {
+ Dn searchBaseDn = new Dn(searchBase);
+ return searchBaseDn.isDescendantOf(proxyBaseDn) ||
+ searchBaseDn.isDescendantOf(remoteBaseDn);
Review Comment:
`new Dn(searchBase)` has no SchemaManager, so RDN values are compared
case-sensitively (api-ldap-model 2.1.8: `Ava.equals` -> `Value.equals` ->
`upValue.equals(...)` when there is no AttributeType). The caller
(`UserSearchInterceptor`) passes `ctx.getDn().toString()`, which is the
user-provided form. So `-b 'ou=people,DC=Proxy,DC=com'`, or a remote base
configured as `DC=Example,DC=com` and queried as `dc=example,dc=com`, is
silently skipped and returns no backend results, even though these DNs are
equal under LDAP matching rules (`dc`/`ou`/`cn` use caseIgnoreMatch).
Suggest a schema-aware Dn (pass the SchemaManager or the normalized
`ctx.getDn()` from `UserSearchInterceptor`), or at least lowercase both sides:
Dn searchBaseDn = new Dn(searchBase.toLowerCase(Locale.ROOT));
return searchBaseDn.isDescendantOf(proxyBaseDn.toLowerCase(Locale.ROOT))
||
searchBaseDn.isDescendantOf(remoteBaseDn.toLowerCase(Locale.ROOT));
A mixed-case base in `testIsSupportedSearchBase` would cover it.
##########
knox-site/docs/service_ldap_server.md:
##########
@@ -232,12 +276,165 @@ knoxcli.sh create-alias my_backend_bind_password --value
<password>
If the alias may be created shortly after the configuration is loaded (for
example when a Kubernetes Secret and the configuration referencing it are
created together), set `gateway.ldap.backend.password.alias.retry.attempts` to
have Knox look up a missing alias again every
`gateway.ldap.backend.password.alias.retry.interval.ms` milliseconds before
giving up ([KNOX-3513](https://issues.apache.org/jira/browse/KNOX-3513)). Each
retry is logged at WARN; the ERROR is only logged once the retries are
exhausted. All missing aliases are retried together, so the LDAP service waits
at most attempts x interval while starting or reloading. The Knox CLI
(`knoxcli.sh`) never retries, so commands such as `create-alias` are not held
up by the missing alias.
+### Role Strategy
+
+The role strategy, selected with `gateway.ldap.roles.lookup.strategy`, is
where the [Roles Lookup Interceptor](#roles-lookup-interceptor-roleslookup)
gets its role data. The same strategy answers three questions:
+
+| Question | Used for |
+| :--- | :--- |
+| Which roles does a user have? (`user` and the user's `groups` -> roles) |
Role mapping of user and group entries |
+| Which roles exist? | Role listing |
+| Which users and groups is a role assigned to? (role -> users and groups) |
Reverse role lookup |
+
+A role is identified by a scope and a name. It is written as `<scope>:<name>`
(for example `platform:admin`) wherever Knox shows or accepts a role, such as
in `memberOf` values, role entry DNs (`cn=platform:admin,ou=groups,<base dn>`)
and role listings. A role without a scope is written as just `<name>`.
+
+#### File-based Roles Strategy (`file`)
+
+The file strategy reads role assignments from a JSON file. It is suited to
testing and small, static environments. Set
`gateway.ldap.roles.lookup.strategy` to `file` and
`gateway.ldap.roles.lookup.file.path` to the path of the file.
+
+The file contains a JSON array. Each element assigns roles to one user or one
group:
+
+| Field | Description |
+| :--- | :--- |
+| `id` | The user name or group name the roles are assigned to. |
+| `type` | `user` or `group`. |
+| `roles` | The list of roles assigned. Each role has a `name` and an optional
`scope`. |
+
+```json
+[
+ {
+ "id": "alice",
+ "type": "user",
+ "roles": [
+ { "scope": "platform", "name": "awc-admin" }
+ ]
+ },
+ {
+ "id": "engineering",
+ "type": "group",
+ "roles": [
+ { "scope": "ml-workspace-abc", "name": "viewer" },
+ { "scope": "platform", "name": "analyst" }
+ ]
+ }
+]
+```
+
+The entries are used as follows:
+
+- **User to roles.** The roles of a user are the roles of every `user` entry
whose `id` equals the user name, plus the roles of every `group` entry whose
`id` is one of the user's groups. Duplicate roles are removed.
+- **Role to assignees.** The assignees of a role are the `id` of every entry
that has the role, split into users (`type` is `user`) and groups (`type` is
`group`). Roles are compared as `<scope>:<name>`, ignoring case.
+- **Listing roles.** The distinct `<scope>:<name>` values of all the entries.
The file format does not support roles that have no assignments.
+
+The same file is used for all three use-cases to ensure consist results. The
file is read on every request, so changes take effect without restarting the
gateway. If the file does not exist or cannot be parsed, the request that needs
it fails with the error `Role mapping file ... does not exist` or `Error
reading roles mapping file`.
+
+#### Roles REST API contract (`rest`)
+
+The REST strategy calls an external service. Set
`gateway.ldap.roles.lookup.strategy` to `rest` and configure all three
endpoints:
+
+| Property | HTTP request | Purpose |
+| :--- | :--- |:---------------------------------------|
+| `gateway.ldap.roles.lookup.rest.api.endpoint` | `POST <endpoint>` | Get the
roles of a user and/or groups. |
+| `gateway.ldap.roles.list.rest.api.endpoint` | `GET
<endpoint>?name=&scope=&offset=` | List roles, one page at a time. |
+| `gateway.ldap.roles.get.rest.api.endpoint` | `GET <endpoint>/<role id>` |
Get one role and its members. |
+
+All responses must have status `200` and a JSON body. Any other status, or an
empty body, makes the request fail and the failure is logged. Unknown JSON
properties in responses are ignored.
+
+##### Look up the roles of a user
+
+Knox sends a `POST` with a JSON body (`Content-Type: application/json`)
containing the username and the names of the user's groups:
+
+```json
+{
+ "user_id": "alice",
+ "groups": ["engineering", "analysts"]
+}
+```
+
+The service responds with the roles that apply to the user and groups. `scope`
may be omitted or empty for a role that has no scope.
+
+```json
+{
+ "user_id": "alice",
+ "roles": [
+ { "scope": "platform", "name": "awc-admin" },
+ { "scope": "ml-workspace-abc", "name": "viewer" }
+ ]
+}
+```
+
+##### List roles
+
+Knox sends a `GET` with the header `Accept: application/json`. The query
parameters are:
+
+| Parameter | Description |
+| :--- | :--- |
+| `offset` | Always sent. The number of roles Knox has already retrieved,
starting at `0`. |
+| `name` | Only sent when Knox looks for one role (reverse role lookup). Only
roles with this name should be returned. |
+| `scope` | Only sent when Knox looks for one role (reverse role lookup) and
the role has a scope. Only roles with this scope should be returned. |
+
+The service responds with a page of roles and the paging information:
+
+```json
+{
+ "data": [
+ { "id": "role-1", "name": "admin", "scope": "platform" },
+ { "id": "role-2", "name": "viewer", "scope": "workspace-abc" }
+ ],
+ "pagination": {
+ "limit": 2,
+ "offset": 0,
+ "total": 5
+ }
+}
+```
+
+| Field | Description |
+| :--- | :--- |
+| `data[].id` | The unique id of the role. It is used to get the role's
members. |
+| `data[].name` | The role name. |
+| `data[].scope` | The role scope. |
+| `pagination.total` | The total number of roles that match the request. |
+
+Knox keeps requesting pages, advancing `offset` by the number of roles it has
received, until it has received `pagination.total` roles. The page size is
chosen by the service. A page with no roles before `total` is reached fails the
listing with `No more roles to list`.
+
+When a single role is looked up, Knox searches the returned roles for the one
whose `name` and `scope` equal the requested role, ignoring case. The service
may therefore return more than the exact match, but it must include the
matching role.
+
+##### Get a role and its members
+
+Knox sends a `GET` with the header `Accept: application/json` to the get
endpoint with the role `id` appended as a path segment. The id is URL-encoded,
with spaces encoded as `%20`. For example, with the endpoint
`http://roles.example.com/roles/get` and the role id `role 1`, Knox requests
`http://roles.example.com/roles/get/role%201`.
+
+The service responds with the role and its members:
+
+```json
+{
+ "id": "role-1",
+ "name": "admin",
+ "scope": "platform",
+ "members": [
+ { "id": "m-1", "member_type": "user", "member_id": "alice" },
+ { "id": "m-2", "member_type": "group", "member_id": "engineering" }
+ ]
+}
+```
+
+| Field | Description |
+| :--- | :--- |
+| `members[].member_type` | `user` or `group` (case-insensitive). Members of
any other type are ignored and logged. |
Review Comment:
"Members of any other type are ignored and logged." no longer matches the
code: `RestApiLdapRolesReverseLookup` now throws a `RoleException` for an
unknown `member_type`, which fails the LDAP search (see
`testLookupRoleAssigneesThrowsOnUnknownMemberType`). Please update the docs to
say an unknown `member_type` fails the lookup and the LDAP search, or switch
the code to log-and-skip if that's the intended contract.
##########
knox-site/docs/service_ldap_server.md:
##########
@@ -140,25 +149,59 @@ The disabled user filter interceptor will recognize and
filter disabled users. I
#### Roles Lookup Interceptor (`rolesLookup`)
-The rolesLookup interceptor is created if the `interceptorType` configuration
is set to `rolesLookup`. This interceptor transforms the response entities
based on the mappings provided by the Role Lookup Service. For each entity, a
request will be made to lookup roles based on the user's name and group
membership. These roles will replace the values in the `memberOf` attribute.
+The rolesLookup interceptor is created if the `interceptorType` configuration
is set to `rolesLookup`. It uses the Role Lookup Service, configured with
`gateway.ldap.roles.lookup.strategy`, to apply roles to search requests in
three ways.
+
+**Role mapping.** For each user entry returned, a request is made to lookup
roles based on the user's name and group membership. These roles replace the
values in the `memberOf` attribute. Group entries are replaced with role
entries named after the roles mapped to the group.
+
+E.g.,
+```shell
+ldapsearch -H ldap://<ldaphost>:<ldapport> -D <bind_dn> -w <bind_password> -b
'ou=people,dc=proxy,dc=com', -s sub '(objectClass=inetOrgPerson)' '*'
+```
+
+**Role listing.** A search for all groups is answered with the roles known to
the Role Lookup Service instead of the backend groups. Each role is returned as
a `groupOfNames` entry with the DN `cn=<scope>:<name>,ou=groups,<base dn>`.
Listed Role entries do not contain the `member` attribute because the role data
is returned through the role strategy without further search requests to LDAP.
A search is an all-groups search when its scope is not `base` (object) and its
filter is an all-groups filter, and it is issued either from `ou=groups,<base
dn>` or from the root of the directory (a base DN of at most two RDNs). A
filter is an all-groups filter if it is one of:
+
+- `(objectClass=groupOfNames)` or `(objectClass=groupOfUniqueNames)`
+- `(cn=*)` or `(objectClass=*)`
+- an AND filter where every child is an all-groups filter, for example
`(&(objectClass=groupOfNames)(cn=*))`
+- an OR filter where at least one child is an all-groups filter
Review Comment:
The all-groups rules in the code changed with the last fixes, but this
paragraph still describes the old ones:
- Line 161, "from the root of the directory (a base DN of at most two
RDNs)": the code now accepts any ancestor of `ou=groups,<base dn>` (compared as
a `Dn`). It no longer counts RDNs.
- Line 164: `(cn=*)` / `(objectClass=*)` are only all-groups filters when
the base is `ou=groups,<base dn>`. From an ancestor they now go to the backend.
- Line 166, "an OR filter where at least one child is an all-groups filter":
this only applies at `ou=groups,<base dn>`. From an ancestor, every OR child
must be `(objectClass=groupOfNames)` or `(objectClass=groupOfUniqueNames)`. An
AND needs at least one such objectClass child, and every other child must be an
all-groups filter.
Suggest splitting the list by search base, for example:
From `ou=groups,<base dn>`: `(objectClass=groupOfNames)`,
`(objectClass=groupOfUniqueNames)`, `(cn=*)`, `(objectClass=*)`, an AND where
every child is one of these, or an OR where at least one child is.
From an ancestor of `ou=groups,<base dn>` (e.g. `<base dn>`):
`(objectClass=groupOfNames)`, `(objectClass=groupOfUniqueNames)`, an OR of only
those, or an AND that contains at least one of them and whose other children
are all-groups filters, e.g. `(&(objectClass=groupOfNames)(cn=*))`.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
