Fossil SCM

logout now fails if the auth token is not available to it (as a sanity check and potentially stop someone from logging out someone else).

stephan 2011-09-17 14:24 UTC json
Commit affdf56c3fef4e57c6a72b55e6e592d929b04eaa
Parent 147f4bfb62bfb94…
2 files changed +33 -11 +11 -1
~ src/json.c ~ src/login.c
M src/json.c
+33 -11
--- src/json.c
+++ src/json.c
@@ -54,10 +54,11 @@
5454
 FSL_JSON_E_ASSERT = FSL_JSON_E_GENERIC_SUB1 + 6,
5555
 FSL_JSON_E_ALLOC = FSL_JSON_E_GENERIC_SUB1 + 7,
5656
 FSL_JSON_E_NYI = FSL_JSON_E_GENERIC_SUB1 + 8,
5757
5858
 FSL_JSON_E_AUTH = 2000,
59
+/* #2001: re-use */
5960
 FSL_JSON_E_MISSING_AUTH = FSL_JSON_E_AUTH + 2,
6061
 FSL_JSON_E_DENIED = FSL_JSON_E_AUTH + 3,
6162
 FSL_JSON_E_WRONG_MODE = FSL_JSON_E_AUTH + 4,
6263
6364
 FSL_JSON_E_LOGIN_FAILED = FSL_JSON_E_AUTH + 100,
@@ -203,12 +204,10 @@
203204
 cson_value * json_rc_string( int code ){
204205
 return cson_value_new_string( json_rc_cstr(code), 11 );
205206
 }
206207
207208
 /*
208
-** UNTESTED!!!
209
-**
210209
 ** Returns the current request's JSON authentication token, or NULL if
211210
 ** none is found. The token's memory is owned by (or shared with)
212211
 ** g.json.cgiCx.
213212
 **
214213
 ** If an auth token is found in the GET/POST JSON request data then
@@ -228,18 +227,25 @@
228227
 JSON, or fossil cookie (in that order). */
229228
 g.json.authToken = cson_cgi_getenv(&g.json.cgiCx, "gp", FossilJsonKeys.authToken)
230229
 /* reminder to self: cson_cgi does not have access to the cookies
231230
 because fossil's core consumes them. Thus we cannot use "agpc"
232231
 here. We use "a" (App-specific) as a place to store fossil's
233
- cookie value.
232
+ cookie value. Reminder #2: in server mode cson_cgi also doesn't
233
+ have access to the GET parameters because of how the QUERY_STRING
234
+ is set. That's on my to-fix list for cson_cgi (feeding it our own
235
+ query string).
234236
 */;
235
- if( g.json.authToken){
237
+ if(g.json.authToken && cson_value_is_string(g.json.authToken)){
236238
 /* tell fossil to use this login info.
237239
238
- FIXME: because the JSON bits don't carry around login_cookie_name(),
239
- there is a login hijacking window here. We need to change the
240
- JSON auth token to be in the form: login_cookie_name()=...
240
+ FIXME: because the JSON bits don't carry around
241
+ login_cookie_name(), there is a potential login hijacking
242
+ window here. We may need to change the JSON auth token to be
243
+ in the form: login_cookie_name()=...
244
+
245
+ Then again, the hardened cookie value helps ensure that
246
+ only a proper key/value match is valid.
241247
 */
242248
 cgi_replace_parameter( login_cookie_name(), cson_value_get_cstr(g.json.authToken) );
243249
 }else if( g.isCGI ){
244250
 /* try fossil's conventional cookie. */
245251
 /* Reminder: chicken/egg scenario regarding db access in CLI
@@ -857,16 +863,32 @@
857863
 }
858864
859865
 /*
860866
 ** Impl of /json/logout.
861867
 **
862
-** Shortcomings: this never reports failure, as we don't go through
863
-** the trouble of actually checking whether the user is logged in or
864
-** not.
865868
 */
866869
 cson_value * json_page_logout(void){
867
- login_clear_login_data();
870
+ cson_value const *token = g.json.authToken;
871
+ /* Remember that json_bootstrap() replaces the login cookie with
872
+ the JSON auth token if the request contains it. If the reqest
873
+ is missing the auth token then this will fetch fossil's
874
+ original cookie. Either way, it's what we want :).
875
+
876
+ We require the auth token to avoid someone maliciously
877
+ trying to log someone else out (not 100% sure if that
878
+ would be possible, given fossil's hardened cookie, but
879
+ i'll assume it would be for the time being).
880
+ */
881
+ ;
882
+ if(!token){
883
+ g.json.resultCode = FSL_JSON_E_MISSING_AUTH;
884
+ }else{
885
+ login_clear_login_data();
886
+ g.json.authToken = NULL /* memory is owned by g.json.cgiCx, but
887
+ now lives only in the cson_cgi garbage
888
+ collector.*/;
889
+ }
868890
 return NULL;
869891
 }
870892
871893
 /*
872894
 ** Implementation of the /json/stat page/command.
873895
--- src/json.c
+++ src/json.c
@@ -54,10 +54,11 @@
54 FSL_JSON_E_ASSERT = FSL_JSON_E_GENERIC_SUB1 + 6,
55 FSL_JSON_E_ALLOC = FSL_JSON_E_GENERIC_SUB1 + 7,
56 FSL_JSON_E_NYI = FSL_JSON_E_GENERIC_SUB1 + 8,
57
58 FSL_JSON_E_AUTH = 2000,
 
59 FSL_JSON_E_MISSING_AUTH = FSL_JSON_E_AUTH + 2,
60 FSL_JSON_E_DENIED = FSL_JSON_E_AUTH + 3,
61 FSL_JSON_E_WRONG_MODE = FSL_JSON_E_AUTH + 4,
62
63 FSL_JSON_E_LOGIN_FAILED = FSL_JSON_E_AUTH + 100,
@@ -203,12 +204,10 @@
203 cson_value * json_rc_string( int code ){
204 return cson_value_new_string( json_rc_cstr(code), 11 );
205 }
206
207 /*
208 ** UNTESTED!!!
209 **
210 ** Returns the current request's JSON authentication token, or NULL if
211 ** none is found. The token's memory is owned by (or shared with)
212 ** g.json.cgiCx.
213 **
214 ** If an auth token is found in the GET/POST JSON request data then
@@ -228,18 +227,25 @@
228 JSON, or fossil cookie (in that order). */
229 g.json.authToken = cson_cgi_getenv(&g.json.cgiCx, "gp", FossilJsonKeys.authToken)
230 /* reminder to self: cson_cgi does not have access to the cookies
231 because fossil's core consumes them. Thus we cannot use "agpc"
232 here. We use "a" (App-specific) as a place to store fossil's
233 cookie value.
 
 
 
234 */;
235 if( g.json.authToken){
236 /* tell fossil to use this login info.
237
238 FIXME: because the JSON bits don't carry around login_cookie_name(),
239 there is a login hijacking window here. We need to change the
240 JSON auth token to be in the form: login_cookie_name()=...
 
 
 
 
241 */
242 cgi_replace_parameter( login_cookie_name(), cson_value_get_cstr(g.json.authToken) );
243 }else if( g.isCGI ){
244 /* try fossil's conventional cookie. */
245 /* Reminder: chicken/egg scenario regarding db access in CLI
@@ -857,16 +863,32 @@
857 }
858
859 /*
860 ** Impl of /json/logout.
861 **
862 ** Shortcomings: this never reports failure, as we don't go through
863 ** the trouble of actually checking whether the user is logged in or
864 ** not.
865 */
866 cson_value * json_page_logout(void){
867 login_clear_login_data();
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
868 return NULL;
869 }
870
871 /*
872 ** Implementation of the /json/stat page/command.
873
--- src/json.c
+++ src/json.c
@@ -54,10 +54,11 @@
54 FSL_JSON_E_ASSERT = FSL_JSON_E_GENERIC_SUB1 + 6,
55 FSL_JSON_E_ALLOC = FSL_JSON_E_GENERIC_SUB1 + 7,
56 FSL_JSON_E_NYI = FSL_JSON_E_GENERIC_SUB1 + 8,
57
58 FSL_JSON_E_AUTH = 2000,
59 /* #2001: re-use */
60 FSL_JSON_E_MISSING_AUTH = FSL_JSON_E_AUTH + 2,
61 FSL_JSON_E_DENIED = FSL_JSON_E_AUTH + 3,
62 FSL_JSON_E_WRONG_MODE = FSL_JSON_E_AUTH + 4,
63
64 FSL_JSON_E_LOGIN_FAILED = FSL_JSON_E_AUTH + 100,
@@ -203,12 +204,10 @@
204 cson_value * json_rc_string( int code ){
205 return cson_value_new_string( json_rc_cstr(code), 11 );
206 }
207
208 /*
 
 
209 ** Returns the current request's JSON authentication token, or NULL if
210 ** none is found. The token's memory is owned by (or shared with)
211 ** g.json.cgiCx.
212 **
213 ** If an auth token is found in the GET/POST JSON request data then
@@ -228,18 +227,25 @@
227 JSON, or fossil cookie (in that order). */
228 g.json.authToken = cson_cgi_getenv(&g.json.cgiCx, "gp", FossilJsonKeys.authToken)
229 /* reminder to self: cson_cgi does not have access to the cookies
230 because fossil's core consumes them. Thus we cannot use "agpc"
231 here. We use "a" (App-specific) as a place to store fossil's
232 cookie value. Reminder #2: in server mode cson_cgi also doesn't
233 have access to the GET parameters because of how the QUERY_STRING
234 is set. That's on my to-fix list for cson_cgi (feeding it our own
235 query string).
236 */;
237 if(g.json.authToken && cson_value_is_string(g.json.authToken)){
238 /* tell fossil to use this login info.
239
240 FIXME: because the JSON bits don't carry around
241 login_cookie_name(), there is a potential login hijacking
242 window here. We may need to change the JSON auth token to be
243 in the form: login_cookie_name()=...
244
245 Then again, the hardened cookie value helps ensure that
246 only a proper key/value match is valid.
247 */
248 cgi_replace_parameter( login_cookie_name(), cson_value_get_cstr(g.json.authToken) );
249 }else if( g.isCGI ){
250 /* try fossil's conventional cookie. */
251 /* Reminder: chicken/egg scenario regarding db access in CLI
@@ -857,16 +863,32 @@
863 }
864
865 /*
866 ** Impl of /json/logout.
867 **
 
 
 
868 */
869 cson_value * json_page_logout(void){
870 cson_value const *token = g.json.authToken;
871 /* Remember that json_bootstrap() replaces the login cookie with
872 the JSON auth token if the request contains it. If the reqest
873 is missing the auth token then this will fetch fossil's
874 original cookie. Either way, it's what we want :).
875
876 We require the auth token to avoid someone maliciously
877 trying to log someone else out (not 100% sure if that
878 would be possible, given fossil's hardened cookie, but
879 i'll assume it would be for the time being).
880 */
881 ;
882 if(!token){
883 g.json.resultCode = FSL_JSON_E_MISSING_AUTH;
884 }else{
885 login_clear_login_data();
886 g.json.authToken = NULL /* memory is owned by g.json.cgiCx, but
887 now lives only in the cson_cgi garbage
888 collector.*/;
889 }
890 return NULL;
891 }
892
893 /*
894 ** Implementation of the /json/stat page/command.
895
M src/login.c
+11 -1
--- src/login.c
+++ src/login.c
@@ -289,15 +289,25 @@
289289
 */
290290
 void login_clear_login_data(){
291291
 if(!g.userUid){
292292
 return;
293293
 }else{
294
+ char const * cookie = login_cookie_name();
294295
 /* To logout, change the cookie value to an empty string */
295
- cgi_set_cookie(login_cookie_name(), "",
296
+ cgi_set_cookie(cookie, "",
296297
 login_cookie_path(), -86400);
297298
 db_multi_exec("UPDATE user SET cookie=NULL, ipaddr=NULL, "
298299
 " cexpire=0 WHERE uid=%d", g.userUid);
300
+ cgi_replace_parameter(cookie, NULL)
301
+ /* At the time of this writing, cgi_replace_parameter() was
302
+ ** "NULL-value-safe", and i'm hoping the NULL doesn't cause any
303
+ ** downstream problems here. We could alternately use "" here.
304
+ */
305
+ ;
306
+ /* Potential improvement: do we want/need to skip this step for
307
+ ** the guest user?
308
+ */
299309
 }
300310
 }
301311
302312
 /*
303313
 ** WEBPAGE: login
304314
--- src/login.c
+++ src/login.c
@@ -289,15 +289,25 @@
289 */
290 void login_clear_login_data(){
291 if(!g.userUid){
292 return;
293 }else{
 
294 /* To logout, change the cookie value to an empty string */
295 cgi_set_cookie(login_cookie_name(), "",
296 login_cookie_path(), -86400);
297 db_multi_exec("UPDATE user SET cookie=NULL, ipaddr=NULL, "
298 " cexpire=0 WHERE uid=%d", g.userUid);
 
 
 
 
 
 
 
 
 
299 }
300 }
301
302 /*
303 ** WEBPAGE: login
304
--- src/login.c
+++ src/login.c
@@ -289,15 +289,25 @@
289 */
290 void login_clear_login_data(){
291 if(!g.userUid){
292 return;
293 }else{
294 char const * cookie = login_cookie_name();
295 /* To logout, change the cookie value to an empty string */
296 cgi_set_cookie(cookie, "",
297 login_cookie_path(), -86400);
298 db_multi_exec("UPDATE user SET cookie=NULL, ipaddr=NULL, "
299 " cexpire=0 WHERE uid=%d", g.userUid);
300 cgi_replace_parameter(cookie, NULL)
301 /* At the time of this writing, cgi_replace_parameter() was
302 ** "NULL-value-safe", and i'm hoping the NULL doesn't cause any
303 ** downstream problems here. We could alternately use "" here.
304 */
305 ;
306 /* Potential improvement: do we want/need to skip this step for
307 ** the guest user?
308 */
309 }
310 }
311
312 /*
313 ** WEBPAGE: login
314

Keyboard Shortcuts

Open search /
Next entry (timeline) j
Previous entry (timeline) k
Open focused entry Enter
Show this help ?
Toggle theme Top nav button