search

ncsa / oa4mp

Open Authorization for MyProxy
https://oa4mp.org/
Other
8 stars 10 forks source link

NPE during device code flow #141

Closed bbockelm closed 3 months ago

bbockelm commented 10 months ago

Spotted this NPE when device code flow is being run:

WARNING: oa4mp:<44.202.212.94> error: null
java.lang.NullPointerException
        at edu.uiuc.ncsa.oa2.servlet.RFC8628AuthorizationServer.doIt(RFC8628AuthorizationServer.java:212)
        at edu.uiuc.ncsa.security.servlet.AbstractServlet.doGet(AbstractServlet.java:222)
        at javax.servlet.http.HttpServlet.service(HttpServlet.java:670)
        at javax.servlet.http.HttpServlet.service(HttpServlet.java:779)
        at org.apache.catalina.core.ApplicationFilterChain.internalDoFilter(ApplicationFilterChain.java:227)

Staring at the line of code a bit, it may be related to #136.

jjg-123 commented 10 months ago

Disagree. The code gets the transaction from the supplied user code, but if there is no such user code, then the transaction is null. Solution is to test for a null transaction and return that the user code is incorrect/not valid. So was the user code correct at that point?

bbockelm commented 10 months ago

Are we looking at the same line? Line 212 in 5.4.1 of that file is this:

https://github.com/ncsa/oa4mp/blob/5.4.1/proxy/src/main/java/edu/uiuc/ncsa/oa2/servlet/RFC8628AuthorizationServer.java#L212

                        MetaDebugUtil debugger = MyProxyDelegationServlet.createDebugger(trans.getOA2Client());
jjg-123 commented 10 months ago

Yes. Line 212 creates a debugger and the NPE is because trans == null. That should be checked on line 211 and if null, an appropriate error message should be issued to the user.