Skip to content

Commit f4426b0

Browse files
wabsonwabson
committed
HttpOAuth2Connector refactoring
 - Improved error handling - how returns error data in a JSON body - Use private method to copy response data over to the real response - Do not auto-create sessions when retrieving current session
1 parent 7626f9c commit f4426b0

File tree

1 file changed

+99
-45
lines changed

1 file changed

+99
-45
lines changed

‎src/main/java/org/sharextras/webscripts/connector/HttpOAuth2Connector.java‎

Lines changed: 99 additions & 45 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,8 @@
11
package org.sharextras.webscripts.connector;
22

33
import java.io.IOException;
4+
import java.io.PrintWriter;
5+
import java.io.StringWriter;
46
import java.util.HashMap;
57
import java.util.Map;
68

@@ -15,13 +17,15 @@
1517
import org.springframework.extensions.surf.exception.ConnectorServiceException;
1618
import org.springframework.extensions.surf.exception.CredentialVaultProviderException;
1719
import org.springframework.extensions.surf.util.FakeHttpServletResponse;
20+
import org.springframework.extensions.webscripts.Format;
1821
import org.springframework.extensions.webscripts.connector.ConnectorContext;
1922
import org.springframework.extensions.webscripts.connector.ConnectorService;
2023
import org.springframework.extensions.webscripts.connector.Credentials;
2124
import org.springframework.extensions.webscripts.connector.HttpConnector;
2225
import org.springframework.extensions.webscripts.connector.RemoteClient;
2326
import org.springframework.extensions.webscripts.connector.Response;
2427
import org.springframework.extensions.webscripts.connector.ResponseStatus;
28+
import org.springframework.extensions.webscripts.json.JSONWriter;
2529

2630
/**
2731
* Connector for connecting to OAuth 2.0-protected resources
@@ -81,103 +85,105 @@ public Response call(String uri, ConnectorContext context, HttpServletRequest re
8185
try
8286
{
8387
Response resp = null;
84-
HttpSession session = req.getSession();
88+
HttpSession session = req.getSession(false); // TODO check session is non-null
8589
if (!hasAccessToken(session))
8690
{
8791
loadTokens(uri, req);
8892
}
89-
if (logger.isDebugEnabled())
90-
logger.debug("Loading resource " + uri + " - first attempt");
93+
9194
if (hasAccessToken(session))
9295
{
93-
context.setCommitResponseOnAuthenticationError(false);
94-
95-
// Wrap the response object, since it gets committed straight away
96+
// Wrap the response object, since it gets committed straight away, and we may need to retry
9697
FakeHttpServletResponse wrappedRes = new FakeHttpServletResponse(res);
98+
99+
// First call
100+
context.setCommitResponseOnAuthenticationError(false);
101+
if (logger.isDebugEnabled())
102+
logger.debug("Loading resource " + uri + " - first attempt");
97103
resp = callInternal(uri, context, req, wrappedRes);
104+
98105
if (logger.isDebugEnabled())
99106
logger.debug("Response status " + resp.getStatus().getCode() + " " + resp.getStatus().getCodeName());
100-
// We could have a cached access token which has been updated in the repo
107+
108+
// We could have a revoked or expired access token cached which has been updated in the repo
101109

102110
if (resp.getStatus().getCode() == ResponseStatus.STATUS_UNAUTHORIZED ||
103111
resp.getStatus().getCode() == ResponseStatus.STATUS_FORBIDDEN)
104112
{
105113
if (logger.isDebugEnabled())
106114
logger.debug("Loading resource " + uri + " - second attempt");
115+
107116
loadTokens(uri, req);
108-
// Retry the operation
117+
118+
// Retry the operation - second call
109119
if (hasAccessToken(session))
110120
{
111121
context.setCommitResponseOnAuthenticationError(true);
112122
try
113123
{
114124
resp = callInternal(uri, context, req, res);
125+
115126
if (logger.isDebugEnabled())
116127
logger.debug("Response status " + resp.getStatus().getCode() + " " + resp.getStatus().getCodeName());
117128
}
118129
catch (Throwable t)
119130
{
120-
logger.error("Encountered error when attempting to reload", t);
131+
writeError(res, ResponseStatus.STATUS_INTERNAL_SERVER_ERROR,
132+
"ERR_CALLOUT",
133+
"Encountered error when attempting to reload",
134+
t);
135+
return null;
121136
}
122137
}
123138
else
124139
{
125-
throw new RuntimeException("No access token is present");
140+
writeError(res, ResponseStatus.STATUS_UNAUTHORIZED,
141+
"NO_TOKEN",
142+
"No access token is present",
143+
null);
144+
return null;
126145
}
127146
}
128147
else
129148
{
130-
res.setStatus(wrappedRes.getStatus());
131-
res.setCharacterEncoding(wrappedRes.getCharacterEncoding());
132-
// Copy headers over
133-
for (Object hdrname : wrappedRes.getHeaderNames())
134-
{
135-
res.setHeader((String) hdrname, (String) wrappedRes.getHeader((String) hdrname));
136-
}
137-
res.getOutputStream().write(wrappedRes.getContentAsByteArray());
138-
res.flushBuffer();
149+
copyResponseContent(wrappedRes, res, true);
139150
}
140151
}
141152
else
142153
{
143-
// TODO Support unauthenticated access if allowed by the connector instance
144-
ResponseStatus status = new ResponseStatus();
145-
status.setCode(ResponseStatus.STATUS_UNAUTHORIZED);
146-
status.setMessage("No access token is present");
147-
resp = new Response(status);
148-
res.setStatus(ResponseStatus.STATUS_UNAUTHORIZED);
149-
//throw new RuntimeException("No access token is present");
154+
writeError(res, ResponseStatus.STATUS_UNAUTHORIZED,
155+
"NO_TOKEN",
156+
"No access token is present",
157+
null);
158+
return null;
150159

151160
}
152161

153162
return resp;
154163
}
155-
// TODO return responses with errors when we are able to
156164
catch (CredentialVaultProviderException e)
157165
{
158-
/*
159-
* ResponseStatus status = new ResponseStatus();
160-
status.setCode(ResponseStatus.STATUS_INTERNAL_SERVER_ERROR);
161-
status.setMessage("Unable to retrieve OAuth credentials from credential vault");
162-
status.setException(e);
163-
return new Response(status);
164-
*/
165-
throw new RuntimeException("Unable to retrieve OAuth credentials from credential vault", e);
166+
writeError(res, ResponseStatus.STATUS_INTERNAL_SERVER_ERROR,
167+
"ERR_CREDENTIALSTORE",
168+
"Unable to load credential store",
169+
e);
170+
return null;
166171
}
167172
catch (ConnectorServiceException e)
168173
{
169-
/*
170-
ResponseStatus status = new ResponseStatus();
171-
status.setCode(ResponseStatus.STATUS_INTERNAL_SERVER_ERROR);
172-
status.setMessage("Unable to access Alfresco connector in order to retrieve OAuth credentials");
173-
status.setException(e);
174-
return new Response(status);
175-
*/
176-
throw new RuntimeException("Unable to access Alfresco connector in order to retrieve OAuth credentials", e);
174+
writeError(res, ResponseStatus.STATUS_INTERNAL_SERVER_ERROR,
175+
"ERR_FETCH_CREDENTIALS",
176+
"Unable to retrieve OAuth credentials from credential vault",
177+
e);
178+
return null;
177179
}
178180
catch (IOException e)
179181
{
180-
throw new RuntimeException("Error encountered copying outputstream", e);
182+
writeError(res, ResponseStatus.STATUS_INTERNAL_SERVER_ERROR,
183+
"ERR_COPY_RESPONSE",
184+
"Error encountered copying outputstream",
185+
e);
186+
return null;
181187
}
182188
}
183189

@@ -190,7 +196,7 @@ protected void loadTokens(String uri, HttpServletRequest request) throws Credent
190196
{
191197
logger.debug("Loading OAuth tokens");
192198

193-
HttpSession session = request.getSession();
199+
HttpSession session = request.getSession(false);
194200
if (session != null)
195201
{
196202
String userId = (String)session.getAttribute(USER_ID);
@@ -217,6 +223,54 @@ protected void loadTokens(String uri, HttpServletRequest request) throws Credent
217223
}
218224
}
219225

226+
private void copyResponseContent(FakeHttpServletResponse source, HttpServletResponse dest, boolean flush) throws IOException
227+
{
228+
dest.setStatus(source.getStatus());
229+
dest.setCharacterEncoding(source.getCharacterEncoding());
230+
// Copy headers over
231+
for (Object hdrname : source.getHeaderNames())
232+
{
233+
dest.setHeader((String) hdrname, (String) source.getHeader((String) hdrname));
234+
}
235+
dest.getOutputStream().write(source.getContentAsByteArray());
236+
if (flush)
237+
{
238+
dest.flushBuffer();
239+
}
240+
}
241+
242+
private void writeError(HttpServletResponse resp, int status, String id, String message, Throwable e)
243+
{
244+
resp.setStatus(status);
245+
resp.setContentType(Format.JSON.mimetype());
246+
try
247+
{
248+
JSONWriter writer = new JSONWriter(resp.getWriter());
249+
writer.startObject();
250+
writer.startValue("error").startObject();
251+
writer.writeValue("id", id);
252+
writer.writeValue("message", message);
253+
if (e != null)
254+
{
255+
writer.startValue("exception").startObject();
256+
writer.writeValue("message", e.getMessage());
257+
StringWriter sw = new StringWriter();
258+
PrintWriter pw = new PrintWriter(sw);
259+
e.printStackTrace(pw);
260+
writer.writeValue("stackTrace", sw.toString());
261+
writer.endObject();
262+
}
263+
writer.endObject();
264+
writer.endObject();
265+
resp.flushBuffer();
266+
}
267+
catch (IOException e1)
268+
{
269+
// Unable to get writer from response
270+
e1.printStackTrace();
271+
}
272+
}
273+
220274
/* (non-Javadoc)
221275
* @see org.alfresco.connector.HttpConnector#stampCredentials(org.alfresco.connector.RemoteClient, org.alfresco.connector.ConnectorContext)
222276
*/

0 commit comments

Comments
 (0)