[
https://issues.apache.org/jira/browse/KNOX-3551?focusedWorklogId=1052665&page=com.atlassian.jira.plugin.system.issuetabpanels:worklog-tabpanel#worklog-1052665
]
ASF GitHub Bot logged work on KNOX-3551:
----------------------------------------
Author: ASF GitHub Bot
Created on: 05/Nov/26 08:53
Start Date: 05/Nov/26 08:53
Worklog Time Spent: 10m
Work Description: smolnar82 opened a new pull request, #1490:
URL: https://github.com/apache/knox/pull/1490
[KNOX-3551](https://issues.apache.org/jira/browse/KNOX-3551) - KnoxIDF:
accepting consent fails with an NPE because the consent state form field is lost
## What changes were proposed in this pull request?
Clicking **Accept** on the KnoxIDF consent page returned HTTP 500 (an NPE in
`KnoxIDFArtifactStore.get`). The consent page sends `state` as a form POST
field. On topologies with an identity-assertion provider,
`IdentityAsserterHttpServletRequestWrapper` reads and re-encodes the form body
in `getInputStream()`, and after that `request.getParameter("state")` returns
`null`, even on the unwrapped request. That `null` then went to the
Caffeine-backed state store, which throws on null keys.
- `AuthorizeResource.consentAccepted` now binds `state` with `@FormParam`
(`@Consumes(APPLICATION_FORM_URLENCODED)`), so it reads the re-encoded body
that Jersey receives.
- A missing or blank `state` now returns 400 `invalid_request`, with an
audit event (`reason=missing_state`), instead of a 500.
- `KnoxIDFArtifactStore.get()` and `remove()` now treat a `null` key as a
miss.
## How was this patch tested?
- New unit test `AuthorizeResourceConsentStateParamTest` checks that a null
or blank `state` gets a 400 `invalid_request`.
- `KnoxIDFArtifactStoreTest` has a new case checking that a null key is a
miss and doesn't throw.
- Manual test on a local gateway: an SSO topology with the `Default`
identity-assertion provider and `knoxidf.auto.consent.enabled=false`. Run
`/authorize`, then click **Accept** on the consent page. The browser should be
redirected to the `redirect_uri` with `code` and `state`, and the audit log
should show `event=consent ... success`.
## Integration Tests
None added. The existing KnoxIDF compose tests run with
`knoxidf.auto.consent.enabled=true`, so they never reach the consent page.
Covering this path would mean driving the HTML consent form through SSO.
Issue Time Tracking
-------------------
Worklog Id: (was: 1052665)
Remaining Estimate: 0h
Time Spent: 10m
> KnoxIDF: accepting consent fails with an NPE because the consent state form
> field is lost
> -----------------------------------------------------------------------------------------
>
> Key: KNOX-3551
> URL: https://issues.apache.org/jira/browse/KNOX-3551
> Project: Apache Knox
> Issue Type: Improvement
> Components: KnoxIDF
> Reporter: Sandor Molnar
> Assignee: Sandor Molnar
> Priority: Major
> Fix For: 3.1.0
>
> Time Spent: 10m
> Remaining Estimate: 0h
>
> In the KnoxIDF authorization code flow, clicking *Accept* on the consent page
> returns HTTP 500:
> {code}
> java.lang.NullPointerException: Cannot invoke "Object.hashCode()" because
> "key" is null
> at com.github.benmanes.caffeine.cache.LocalManualCache.getIfPresent(...)
> at
> org.apache.knox.gateway.util.knoxidf.KnoxIDFArtifactStore.get(KnoxIDFArtifactStore.java:41)
> at
> org.apache.knox.gateway.service.knoxidf.AuthorizeResource.consentAccepted(AuthorizeResource.java:405)
> {code}
> {{AuthConsentServlet}} sends the consent {{state}} as a hidden field in a
> form POST ({{application/x-www-form-urlencoded}}).
> {{AuthorizeResource.consentAccepted()}} reads it with
> {{request.getParameter("state")}}. When the topology uses an
> identity-assertion provider (e.g. {{Default}}), the request is wrapped by
> {{IdentityAsserterHttpServletRequestWrapper}}. That wrapper reads and
> re-encodes form bodies in {{getInputStream()}}, and once the body has been
> read the container stops exposing form fields as servlet parameters. So
> {{state}} comes back {{null}}, even from the unwrapped request. The {{null}}
> then goes to the Caffeine-backed state store, which throws on null keys.
> *Steps to reproduce:*
> # Use a KnoxIDF SSO topology with an identity-assertion provider and
> {{knoxidf.auto.consent.enabled=false}}.
> # Start the flow at
> {{/knoxidf/api/v1/authorize?response_type=code&client_id=...&state=...&scope=openid&redirect_uri=...}}.
> # Click *Accept* on the consent page. The response is a 500 with the NPE
> above.
> *Recommended fix:*
> * Bind {{state}} in {{consentAccepted}} with JAX-RS {{@FormParam}} instead of
> {{request.getParameter()}}.
> * Return a 400 {{invalid_request}} when {{state}} is missing or blank.
> * Make {{KnoxIDFArtifactStore.get()}} and {{remove()}} treat a null key as a
> miss.
--
This message was sent by Atlassian Jira
(v10.3.26#10030026)
