Closed Bug 1139257 Opened 9 years ago Closed 9 years ago

allow cookie+api-token GET REST requests

Categories

(Bugzilla :: WebService, enhancement)

enhancement
Not set
normal

Tracking

()

RESOLVED FIXED
Bugzilla 6.0

People

(Reporter: glob, Assigned: glob)

References

Details

(Keywords: relnote)

Attachments

(1 file, 1 obsolete file)

currently we don't have any way for javascript which is part of the bugzilla UI to make an authenticated GET REST request.

the rest server has the following comment:

# If we're being called using GET, we don't allow cookie-based or Env
# login, because GET requests can be done cross-domain, and we don't
# want private data showing up on another site unless the user
# explicitly gives that site their username and password. (This is
# particularly important for JSONP, which would allow a remote site
# to use private data without the user's knowledge, unless we had this
# protection in place.)

this stems from bug 550732:

> Both Cookie and Env auth are denied during GET requests (for future
> JSONP protection and because I just don't like the idea of external
> websites being able to get private bug data even now when they can't
> do anything with it)

imho this is an unnecessary restriction, and is preventing us from migrating internal code to the REST api.

using sessions/cookies for internal requests is the standard mechanism of authenticating internal requests (ie. most other frameworks operate this way).


we should allow any REST request to authenticate using cookie + api-token.
extract the user-id out of the cookie and use it to validate the api-token.
Attached patch 1139257_1.patch (obsolete) — Splinter Review
Attachment #8572410 - Flags: review?(dkl)
Attached patch 1139257_2.patchSplinter Review
the first patch doesn't _require_ an api-token.

this revision will drop down to an unauthenticated request when doing a REST request using cookie auth but not api-token.

note: i haven't restricted this logic to GET requests only; i'm not sure if this is the right thing to do.
Attachment #8572410 - Attachment is obsolete: true
Attachment #8572410 - Flags: review?(dkl)
Attachment #8572417 - Flags: review?(dkl)
Comment on attachment 8572417 [details] [diff] [review]
1139257_2.patch

Review of attachment 8572417 [details] [diff] [review]:
-----------------------------------------------------------------

Works as expected. r=dkl

Here is the patch I used for testing if curious:

diff --git a/js/field.js b/js/field.js
index 167ab49..1343287 100644
--- a/js/field.js
+++ b/js/field.js
@@ -808,26 +808,14 @@ function browserCanHideOptions(aSelect) {
  * The Autoselect
  */
 YAHOO.bugzilla.userAutocomplete = {
-    counter : 0,
     dataSource : null,
-    generateRequest : function ( enteredText ){ 
-      YAHOO.bugzilla.userAutocomplete.counter = 
-                                   YAHOO.bugzilla.userAutocomplete.counter + 1;
-      YAHOO.util.Connect.setDefaultPostHeader('application/json', true);
-      var json_object = {
-          method : "User.get",
-          id : YAHOO.bugzilla.userAutocomplete.counter,
-          params : [ { 
-            Bugzilla_api_token: BUGZILLA.api_token,
-            match : [ decodeURIComponent(enteredText) ],
-            include_fields : [ "name", "real_name" ]
-          } ]
-      };
-      var stringified =  YAHOO.lang.JSON.stringify(json_object);
-      var debug = { msg: "json-rpc obj debug info", "json obj": json_object, 
-                    "param" : stringified}
+    generateRequest : function ( enteredText ) {
+      var params = "Bugzilla_api_token=" + BUGZILLA.api_token +
+                   "&match=" + decodeURIComponent(enteredText) +
+                   "&include_fields=name,real_name";
+      var debug = { msg: "rest debug info", "params" : params };
       YAHOO.bugzilla.userAutocomplete.debug_helper( debug );
-      return stringified;
+      return params;
     },
     resultListFormat : function(oResultData, enteredText, sResultMatch) {
         return ( YAHOO.lang.escapeHTML(oResultData.real_name) + " ("
@@ -839,16 +827,17 @@ YAHOO.bugzilla.userAutocomplete = {
             console.log("debug helper info:", arguments);
         }
         return true;
-    },    
+    },
     init_ds : function(){
-        this.dataSource = new YAHOO.util.XHRDataSource("jsonrpc.cgi");
+        YAHOO.util.Connect.initHeader('Accept', 'application/json', true);
+        this.dataSource = new YAHOO.util.XHRDataSource("rest.cgi/user?");
+        this.dataSource.responseType = YAHOO.util.XHRDataSource.TYPE_JSON;
         this.dataSource.connTimeout = 30000;
-        this.dataSource.connMethodPost = true;
         this.dataSource.connXhrMode = "cancelStaleRequests";
         this.dataSource.maxCacheEntries = 5;
         this.dataSource.responseSchema = {
-            resultsList : "result.users",
-            metaFields : { error: "error", jsonRpcId: "id"},
+            resultsList : "users",
+            metaFields : { error: "error" },
             fields : [
                 { key : "name" },
                 { key : "real_name"}
@@ -857,9 +846,9 @@ YAHOO.bugzilla.userAutocomplete = {
     },
     init : function( field, container, multiple ) {
         if( this.dataSource == null ){
-            this.init_ds();  
-        }            
-        var userAutoComp = new YAHOO.widget.AutoComplete( field, container, 
+            this.init_ds();
+        }
+        var userAutoComp = new YAHOO.widget.AutoComplete( field, container,
                                 this.dataSource );
         // other stuff we might want to do with the autocomplete goes here
         userAutoComp.maxResultsDisplayed = BUGZILLA.param.maxusermatches;
@@ -876,7 +865,6 @@ YAHOO.bugzilla.userAutocomplete = {
         if( multiple == true ){
             userAutoComp.delimChar = [","];
         }
-        
     }
 };
Attachment #8572417 - Flags: review?(dkl) → review+
Flags: approval?
on irc lpsolit asked why the login_cookie is being cleared.

as per bug 726696, all authenicated webservice calls should require a token for authentication, so we cannot allow just cookie auth to succeed.

in other words, if a rest request includes a cookie but not an api-token we need to treat that as not-authenticated.  by first revision didn't do this -- you could make authenticated rest requests without an api-token as long as you provided a valid session cookie.

in order to prevent cookie-only auth, the cookie is removed from token-less rest requests.  the net effect is "ignore the login cookie on rest requests unless there's also an api-token".
To ssh://gitolite3@git.mozilla.org/bugzilla/bugzilla.git
   4a900ca..243d66a  master -> master
Status: NEW → RESOLVED
Closed: 9 years ago
Flags: approval? → approval+
Resolution: --- → FIXED
Keywords: relnote
Blocks: 1268989
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Creator:
Created:
Updated:
Size: