qingstor / qingstor-sdk-python

The official QingStor SDK for the Python programming language.
https://qingstor-sdk-python.readthedocs.io/en/latest/
Apache License 2.0
13 stars 12 forks source link

your signature is not canonical signature stated in the latest document #7

Closed yiakwy closed 7 years ago

yiakwy commented 7 years ago

you can refer to my implementation in python: https://github.com/yiakwy/Siganture-Authentication-Package/blob/master/SDK/sign5py3k.py

in client side:

Request.py

(1) in Authorization part, it should start with "Signature" so that other programmes can know at which part of authorization process you are in.

(2) You did not specify which encoding algorithms you are using. This should be a serious bug in the future. Optionally, you should specify which headers you want to use to encode for a signature. 403 response will tell users how to use headers.

(3) signature template should be : signature_string = "(request-target): {method} {path}\n{ret}" (notice to ascii space)

and path should be included in signature_string template. The path does not include parameters otherwise for some url, the result of which will exceed 128 bytes.

config.py

the best practice should be:

a global_config.py for system. Then use dir to open a user's local config and replace the default values with ones defined by a user. I noticed that you use yaml. But in python, .py is just enough because it is a dynamic language. Using .py, you can implement "Lazy loading mode" very easily.

Architecture

As a SDK, I suggest the architecture as following:

Config & authentication

Global_config.py

Auth.py

  1. SignatureAuthentication(Authentication)

Qingstor.py

  1. class QingstorAuthentication(SignatureAuthentication):
      def __init__(app_key, app_secret, permi=(), timeout=60)
  2. class Application:
     def make_req(method, url, params, headers, timeout)
           # use httpConnection, Reqeust, Oauth whatever method you like ,,, haha
  3. class QingstorApplication(Application):
     # business logics
     def get_xx
     def get_yy
     def get_zz
  4. Constants should be defined in __init__.py; you can also import other files in __init__.py

  5. Do to abuse to use get_xx method, if you just want to get attribute internally , access it directly or use _get_xx method instead.

error.py

  1. I suggest to define a handle_error method in "exception.py" or "error.py"
def handler_errors(func)

    logger =  loggerAdapter(_log, func.__name__)

    def wrapper(*args, **kw):
         try:
             ret = func(*args, **kw)
         except xx:
         except yy:
         except zz:
             logger.WHAT[ex]()
             # return None or raise an error as you wish
    return wrapper
Xuanwo commented 7 years ago

for Request.py: Our signature depends on our server side implementation, you can access infomation here: https://docs.qingcloud.com/qingstor/api/common/signature.html

for config.py: Our sdk is used to access our qingstor service, we should follow the same behavior for all sdk. As for now, all the sdk and cli commands tools will use config.yaml in ~/.qingstor. We will take your advice in consideration for next major version of our API.

for Architecture: I don't think it's right to hava a make_req function, it's hard for developer to use our API, with snips help, we can generate our code easily. And for constants, place them into __init__.py may be a good idea, I will take it into consideration. For get_xx, it's just my own code taste, I don't like these style of functions, so I will not take this. And it the same for handler_errors, I hope you will realize.

Finally, thank you for your advice, wish your happiness while using our sdk and service.

yiakwy commented 7 years ago

@xuanwo

Hi, thanks for replying! Happy Chinese New Year!

I totally understand the excuse.

For Signature part, considering a complex oauth process expanded:

  1. first round, user sign the request, the server detect the request "Authorization" starts with "QS"
  2. then server send a temporal "code" (lives for at most 10 seconds, in case of attacks)
  3. user utilize "the code" to proceed to "token authentication" (if a http(s)? connection established. I havn't study "web socket" deeply yet.)

Hence comes to the conclusion, if you implement composite of authentication, you need "several bytes" in the front of the "Authorization".

For Architecture part

Most open source algorithm will implement Request as a subclass of "Dictionary". Believe it or not, you can check it. "get_{resource}" is not needed. It is should be implemented as "code gen"

Here is a function template

unreigester_method = """
def get_{resource}(self, {key}={defulat_val} ... )
      return getattr(self, "{key}", {default_val})

Use "reflection" in python, by parsing self you can generate a thousands of attributes method

or

we define as properties, which corresponds to c++ "getter" and "setter", provided that you use Python-C api.

Application.make_req is just about "client request construction". It is a virtual layer never be used

class Application:
    __abcabstract__ = True

It should contain all the methods to maintain a http(s) or socket connection status. Mostly they are about app_key and app_secret.

What I really feel is "you have business logices mixed in one file Bucket"

What if you define QingStorApplicaiton as a service Manager controlling ResourceApplication?

each ResourceApplication is responsible for one resource with CURD operations?


# map 'create' to 'apply_memory'
# in REST protocol , Memory has an operation to create
class Bucket(Application)
   def create():
   def get():
   def put():

class BucketManager(Manager)

   def __new__():
         return Bucket or Bucket List
   def create()
         self.resources.bucket.create()

class QingStor():
    buckets = BucketManager()

QingStor.buckets.create()

Why "get_xx" "_get_xx" important

In "reflection" process, many people will throw out all method names starting with "_" . Hence it is important.

Thanks for responding. Hope to hear from you soon!

Xuanwo commented 7 years ago

Maybe I found the big difference between you and me.

I believe there are thousands of ways to write a sdk for a service, and many of them will be better than mine. But , this sdk was wrote for a production service, that means unless we refactor the whole service and release our brand new API, we should follow the service's behavior instead of asking server send a temporal "code" .

And it may be a good idea to have a code gen for user to get all the data, but you can see, our release is 2.0.2 now, we should be responsible for our users, we can't break our API for a production release.

Please believe that we have tried all the possibilities to make the best sdk we can write. And why qingstor-sdk-python is the way it is now, there are many complex reasons. Maybe you can write one to prove that you consider more comprehensive :smile: