* BREAKING CHANGES, READ CHANGELOG!

* Change Client to be an interface * Add Clone and Close methods to Storage * Add Close method to Response and required it to be closed with defer on use
JiesenSun · Jun 26, 2014 · cca734b · cca734b
1 parent 1132b9a
commit cca734b
Show file tree

Hide file tree

Showing 19 changed files with 222 additions and 66 deletions.
diff --git a/CHANGELOG b/CHANGELOG
@@ -0,0 +1,19 @@
+2014-06-25
+==========
+* BREAKING CHANGES:
+	- Storage interface has 2 new methods, Clone and Close, to better support storages
+	  that need to clone / close in each connection (mgo)
+	- Client was changed to be an interface instead of an struct. Because of that,
+	  the Storage interface also had to change, as interface is already a pointer.
+
+	- HOW TO FIX YOUR CODE:
+		+ In your Storage, add a Clone function returning itself, and a do nothing Close.
+		+ In your Storage, replace all *osin.Client with osin.Client (remove the pointer reference)
+		+ If you used the osin.Client struct directly in your code, change it to osin.DefaultClient,
+		  which is a struct with the same fields that implements the interface.
+		+ Change all accesses using osin.Client to use the methods instead of the fields directly.
+		+ You MUST defer Response.Close in all your http handlers, otherwise some
+		  Storages may not clean correctly.
+
+				resp := server.NewResponse()
+				defer resp.Close()
diff --git a/README.md b/README.md
@@ -26,6 +26,8 @@ server := osin.NewServer(osin.NewServerConfig(), &TestStorage{})
 // Authorization code endpoint
 http.HandleFunc("/authorize", func(w http.ResponseWriter, r *http.Request) {
 	resp := server.NewResponse()
+	defer resp.Close()
+
 	if ar := server.HandleAuthorizeRequest(resp, r); ar != nil {
 
 		// HANDLE LOGIN PAGE HERE
@@ -39,6 +41,8 @@ http.HandleFunc("/authorize", func(w http.ResponseWriter, r *http.Request) {
 // Access token endpoint
 http.HandleFunc("/token", func(w http.ResponseWriter, r *http.Request) {
 	resp := server.NewResponse()
+	defer resp.Close()
+
 	if ar := server.HandleAccessRequest(resp, r); ar != nil {
 		ar.Authorized = true
 		server.FinishAccessRequest(resp, r, ar)
@@ -64,3 +68,26 @@ The code is licensed using "New BSD" license.
 ### Author
 
 Rangel Reale
+rangelreale@gmail.com
+
+### Changes
+
+2014-06-25
+==========
+* BREAKING CHANGES:
+	- Storage interface has 2 new methods, Clone and Close, to better support storages
+	  that need to clone / close in each connection (mgo)
+	- Client was changed to be an interface instead of an struct. Because of that,
+	  the Storage interface also had to change, as interface is already a pointer.
+
+	- HOW TO FIX YOUR CODE:
+		+ In your Storage, add a Clone function returning itself, and a do nothing Close.
+		+ In your Storage, replace all *osin.Client with osin.Client (remove the pointer reference)
+		+ If you used the osin.Client struct directly in your code, change it to osin.DefaultClient,
+		  which is a struct with the same fields that implements the interface.
+		+ Change all accesses using osin.Client to use the methods instead of the fields directly.
+		+ You MUST defer Response.Close in all your http handlers, otherwise some
+		  Storages may not clean correctly.
+		
+				resp := server.NewResponse()
+				defer resp.Close()
diff --git a/access.go b/access.go
@@ -22,7 +22,7 @@ const (
 type AccessRequest struct {
 	Type          AccessRequestType
 	Code          string
-	Client        *Client
+	Client        Client
 	AuthorizeData *AuthorizeData
 	AccessData    *AccessData
 	RedirectUri   string
@@ -48,7 +48,7 @@ type AccessRequest struct {
 // AccessData represents an access grant (tokens, expiration, client, etc)
 type AccessData struct {
 	// Client information
-	Client *Client
+	Client Client
 
 	// Authorize data, for authorization code
 	AuthorizeData *AuthorizeData
@@ -158,13 +158,13 @@ func (s *Server) handleAuthorizationCodeRequest(w *Response, r *http.Request) *A
 	}
 
 	// must have a valid client
-	if ret.Client = getClient(auth, s.Storage, w); ret.Client == nil {
+	if ret.Client = getClient(auth, w.Storage, w); ret.Client == nil {
 		return nil
 	}
 
 	// must be a valid authorization code
 	var err error
-	ret.AuthorizeData, err = s.Storage.LoadAuthorize(ret.Code)
+	ret.AuthorizeData, err = w.Storage.LoadAuthorize(ret.Code)
 	if err != nil {
 		w.SetError(E_INVALID_GRANT, "")
 		w.InternalError = err
@@ -178,7 +178,7 @@ func (s *Server) handleAuthorizationCodeRequest(w *Response, r *http.Request) *A
 		w.SetError(E_UNAUTHORIZED_CLIENT, "")
 		return nil
 	}
-	if ret.AuthorizeData.Client.RedirectUri == "" {
+	if ret.AuthorizeData.Client.GetRedirectUri() == "" {
 		w.SetError(E_UNAUTHORIZED_CLIENT, "")
 		return nil
 	}
@@ -188,16 +188,16 @@ func (s *Server) handleAuthorizationCodeRequest(w *Response, r *http.Request) *A
 	}
 
 	// code must be from the client
-	if ret.AuthorizeData.Client.Id != ret.Client.Id {
+	if ret.AuthorizeData.Client.GetId() != ret.Client.GetId() {
 		w.SetError(E_INVALID_GRANT, "")
 		return nil
 	}
 
 	// check redirect uri
 	if ret.RedirectUri == "" {
-		ret.RedirectUri = ret.Client.RedirectUri
+		ret.RedirectUri = ret.Client.GetRedirectUri()
 	}
-	if err = ValidateUri(ret.Client.RedirectUri, ret.RedirectUri); err != nil {
+	if err = ValidateUri(ret.Client.GetRedirectUri(), ret.RedirectUri); err != nil {
 		w.SetError(E_INVALID_REQUEST, "")
 		w.InternalError = err
 		return nil
@@ -238,13 +238,13 @@ func (s *Server) handleRefreshTokenRequest(w *Response, r *http.Request) *Access
 	}
 
 	// must have a valid client
-	if ret.Client = getClient(auth, s.Storage, w); ret.Client == nil {
+	if ret.Client = getClient(auth, w.Storage, w); ret.Client == nil {
 		return nil
 	}
 
 	// must be a valid refresh code
 	var err error
-	ret.AccessData, err = s.Storage.LoadRefresh(ret.Code)
+	ret.AccessData, err = w.Storage.LoadRefresh(ret.Code)
 	if err != nil {
 		w.SetError(E_INVALID_GRANT, "")
 		w.InternalError = err
@@ -258,13 +258,13 @@ func (s *Server) handleRefreshTokenRequest(w *Response, r *http.Request) *Access
 		w.SetError(E_UNAUTHORIZED_CLIENT, "")
 		return nil
 	}
-	if ret.AccessData.Client.RedirectUri == "" {
+	if ret.AccessData.Client.GetRedirectUri() == "" {
 		w.SetError(E_UNAUTHORIZED_CLIENT, "")
 		return nil
 	}
 
 	// client must be the same as the previous token
-	if ret.AccessData.Client.Id != ret.Client.Id {
+	if ret.AccessData.Client.GetId() != ret.Client.GetId() {
 		w.SetError(E_INVALID_CLIENT, "")
 		w.InternalError = errors.New("Client id must be the same from previous token")
 		return nil
@@ -305,12 +305,12 @@ func (s *Server) handlePasswordRequest(w *Response, r *http.Request) *AccessRequ
 	}
 
 	// must have a valid client
-	if ret.Client = getClient(auth, s.Storage, w); ret.Client == nil {
+	if ret.Client = getClient(auth, w.Storage, w); ret.Client == nil {
 		return nil
 	}
 
 	// set redirect uri
-	ret.RedirectUri = ret.Client.RedirectUri
+	ret.RedirectUri = ret.Client.GetRedirectUri()
 
 	return ret
 }
@@ -331,12 +331,12 @@ func (s *Server) handleClientCredentialsRequest(w *Response, r *http.Request) *A
 	}
 
 	// must have a valid client
-	if ret.Client = getClient(auth, s.Storage, w); ret.Client == nil {
+	if ret.Client = getClient(auth, w.Storage, w); ret.Client == nil {
 		return nil
 	}
 
 	// set redirect uri
-	ret.RedirectUri = ret.Client.RedirectUri
+	ret.RedirectUri = ret.Client.GetRedirectUri()
 
 	return ret
 }
@@ -365,12 +365,12 @@ func (s *Server) handleAssertionRequest(w *Response, r *http.Request) *AccessReq
 	}
 
 	// must have a valid client
-	if ret.Client = getClient(auth, s.Storage, w); ret.Client == nil {
+	if ret.Client = getClient(auth, w.Storage, w); ret.Client == nil {
 		return nil
 	}
 
 	// set redirect uri
-	ret.RedirectUri = ret.Client.RedirectUri
+	ret.RedirectUri = ret.Client.GetRedirectUri()
 
 	return ret
 }
@@ -409,23 +409,23 @@ func (s *Server) FinishAccessRequest(w *Response, r *http.Request, ar *AccessReq
 		}
 
 		// save access token
-		if err = s.Storage.SaveAccess(ret); err != nil {
+		if err = w.Storage.SaveAccess(ret); err != nil {
 			w.SetError(E_SERVER_ERROR, "")
 			w.InternalError = err
 			return
 		}
 
 		// remove authorization token
 		if ret.AuthorizeData != nil {
-			s.Storage.RemoveAuthorize(ret.AuthorizeData.Code)
+			w.Storage.RemoveAuthorize(ret.AuthorizeData.Code)
 		}
 
 		// remove previous access token
 		if ret.AccessData != nil {
 			if ret.AccessData.RefreshToken != "" {
-				s.Storage.RemoveRefresh(ret.AccessData.RefreshToken)
+				w.Storage.RemoveRefresh(ret.AccessData.RefreshToken)
 			}
-			s.Storage.RemoveAccess(ret.AccessData.AccessToken)
+			w.Storage.RemoveAccess(ret.AccessData.AccessToken)
 		}
 
 		// output data
@@ -447,7 +447,7 @@ func (s *Server) FinishAccessRequest(w *Response, r *http.Request, ar *AccessReq
 
 // getClient looks up and authenticates the basic auth using the given
 // storage. Sets an error on the response if auth fails or a server error occurs.
-func getClient(auth *BasicAuth, storage Storage, w *Response) *Client {
+func getClient(auth *BasicAuth, storage Storage, w *Response) Client {
 	client, err := storage.GetClient(auth.Username)
 	if err != nil {
 		w.SetError(E_SERVER_ERROR, "")
@@ -458,11 +458,11 @@ func getClient(auth *BasicAuth, storage Storage, w *Response) *Client {
 		w.SetError(E_UNAUTHORIZED_CLIENT, "")
 		return nil
 	}
-	if client.Secret != auth.Password {
+	if client.GetSecret() != auth.Password {
 		w.SetError(E_UNAUTHORIZED_CLIENT, "")
 		return nil
 	}
-	if client.RedirectUri == "" {
+	if client.GetRedirectUri() == "" {
 		w.SetError(E_UNAUTHORIZED_CLIENT, "")
 		return nil
 	}

diff --git a/authorize.go b/authorize.go
@@ -17,7 +17,7 @@ const (
 // Authorize request information
 type AuthorizeRequest struct {
 	Type        AuthorizeRequestType
-	Client      *Client
+	Client      Client
 	Scope       string
 	RedirectUri string
 	State       string
@@ -36,7 +36,7 @@ type AuthorizeRequest struct {
 // Authorization data
 type AuthorizeData struct {
 	// Client information
-	Client *Client
+	Client Client
 
 	// Authorization code
 	Code string
@@ -110,7 +110,7 @@ func (s *Server) handleCodeRequest(w *Response, r *http.Request) *AuthorizeReque
 	}
 
 	// must have a valid client
-	ret.Client, err = s.Storage.GetClient(r.Form.Get("client_id"))
+	ret.Client, err = w.Storage.GetClient(r.Form.Get("client_id"))
 	if err != nil {
 		w.SetErrorState(E_SERVER_ERROR, "", ret.State)
 		w.InternalError = err
@@ -120,19 +120,19 @@ func (s *Server) handleCodeRequest(w *Response, r *http.Request) *AuthorizeReque
 		w.SetErrorState(E_UNAUTHORIZED_CLIENT, "", ret.State)
 		return nil
 	}
-	if ret.Client.RedirectUri == "" {
+	if ret.Client.GetRedirectUri() == "" {
 		w.SetErrorState(E_UNAUTHORIZED_CLIENT, "", ret.State)
 		return nil
 	}
 
 	// force redirect response to client redirecturl first
-	w.SetRedirect(ret.Client.RedirectUri)
+	w.SetRedirect(ret.Client.GetRedirectUri())
 
 	// check redirect uri
 	if ret.RedirectUri == "" {
-		ret.RedirectUri = ret.Client.RedirectUri
+		ret.RedirectUri = ret.Client.GetRedirectUri()
 	}
-	if err = ValidateUri(ret.Client.RedirectUri, ret.RedirectUri); err != nil {
+	if err = ValidateUri(ret.Client.GetRedirectUri(), ret.RedirectUri); err != nil {
 		w.SetErrorState(E_INVALID_REQUEST, "", ret.State)
 		w.InternalError = err
 		return nil
@@ -158,7 +158,7 @@ func (s *Server) handleTokenRequest(w *Response, r *http.Request) *AuthorizeRequ
 	}
 
 	// must have a valid client
-	ret.Client, err = s.Storage.GetClient(r.Form.Get("client_id"))
+	ret.Client, err = w.Storage.GetClient(r.Form.Get("client_id"))
 	if err != nil {
 		w.SetErrorState(E_SERVER_ERROR, "", ret.State)
 		w.InternalError = err
@@ -168,19 +168,19 @@ func (s *Server) handleTokenRequest(w *Response, r *http.Request) *AuthorizeRequ
 		w.SetErrorState(E_UNAUTHORIZED_CLIENT, "", ret.State)
 		return nil
 	}
-	if ret.Client.RedirectUri == "" {
+	if ret.Client.GetRedirectUri() == "" {
 		w.SetErrorState(E_UNAUTHORIZED_CLIENT, "", ret.State)
 		return nil
 	}
 
 	// force redirect response to client redirecturl first
-	w.SetRedirect(ret.Client.RedirectUri)
+	w.SetRedirect(ret.Client.GetRedirectUri())
 
 	// check redirect uri
 	if ret.RedirectUri == "" {
-		ret.RedirectUri = ret.Client.RedirectUri
+		ret.RedirectUri = ret.Client.GetRedirectUri()
 	}
-	if err = ValidateUri(ret.Client.RedirectUri, ret.RedirectUri); err != nil {
+	if err = ValidateUri(ret.Client.GetRedirectUri(), ret.RedirectUri); err != nil {
 		w.SetErrorState(E_INVALID_REQUEST, "", ret.State)
 		w.InternalError = err
 		return nil
@@ -238,7 +238,7 @@ func (s *Server) FinishAuthorizeRequest(w *Response, r *http.Request, ar *Author
 			ret.Code = code
 
 			// save authorization token
-			if err = s.Storage.SaveAuthorize(ret); err != nil {
+			if err = w.Storage.SaveAuthorize(ret); err != nil {
 				w.SetErrorState(E_SERVER_ERROR, "", ar.State)
 				w.InternalError = err
 				return