Skyspark needs post requests instead of get requests for his_read

[ChristianTremblay]

Let see how we can figure this out

@justindarcy @sjlongland I will need your input on that.

From what I remember, all implementations were using the same base (get requests). Question will be do we need to rewrite everything or create something else exclusive for Skyspark...

[sjlongland]

On 21/8/20 3:04 am, Christian Tremblay wrote:

From what I remember, all implementations were using the same base (get requests). Question will be do we need to rewrite everything or create something else exclusive for Skyspark...

I think we can safely do POST requests for all… so maybe an option is to replace the implementation of _get_grid with code that does a POST with a single-row grid containing what was once query arguments.

Been a long time since I looked at pyhaystack code, and work seems to be moving away from Python as a language in favour of JavaScript.

[ChristianTremblay]

Basic Idea here. I just override _on_his_read for skyspark session. Why breaking every other implementation for now ? :)

def _on_his_read(self, point, rng, callback, **kwargs):
        """
        Skyspark will not accept GET request for his_read by default
        [ref : https://project-haystack.org/forum/topic/787#c6]
            The default behavior of SkySpark is now to disallow GET requests 
            non-idempotent operations. So its still allowed on certain operations 
            such as about, formats, read. However as Chris said it can be toggled 
            back on using Settings|API for backward compatibility.

            However as a recommendation I think we should always be using POST as 
            a safer alternative. Using GET for ops with side-effects is against 
            the HTTP spec. Plus it is an attack vector if cookies are involved. 
            And it provides a more precise way to pass the request payload.

            Its not really from a theoretical perspective. But in SkySpark 
            we allow customers to generate histories using their own custom 
            functions. So from a security perspective we took the safest route 
            and consider it to potentially have side effects.
            If your code is all using GET, then just have the customer set 
            Settings|API allowGetWithSideEffects flag to false and it should all work.
        """
        if isinstance(rng, slice):
            str_rng = ",".join([hszinc.dump_scalar(p) for p in (rng.start, rng.stop)])
        elif not isinstance(rng, string_types):
            str_rng = hszinc.dump_scalar(rng)
        else:
            # Better be valid!
            str_rng = rng

        col_list = [('id',[]),('range',[])]
        his_grid = hszinc.grid.Grid(columns=col_list)
        his_grid.insert(0,{"id": self._obj_to_ref(point), "range": str_rng})
        print(his_grid)
        return self._post_grid(
            "hisRead",
            his_grid,
            callback,
            **kwargs
        )

I think I got it right (not sure my Grid is number 1 but it seem to work) ... but server throws : HTTPError: 400 Client Error: SkyArc-Attest-Key required for url: https://demo.skyfoundry.com/api/demo/hisRead This confirms what @clarsen was seing in issue #84

Reading through forums and doc, i see this

If you are not using a browser, then don't use cookies! Use a bearer token with authorization header

Then my knowledge is not strong enough to overpass this. I don't get it. @sjlongland if you can bring some light it would be awesome.

I'm also poking people that took part of issue #84 @clarsen you were near something in april, maybe you can help @gmanohar25
@san42 @justindarcy
@dawcurious

[clarsen]

The patch I submitted has been running successfully for the last months.

On Sat, Aug 22, 2020 at 8:18 PM Christian Tremblay notifications@github.com wrote:

Basic Idea here.

I just override _on_his_read for skyspark session.

Why breaking every other implementation for now ? :)

def _on_his_read(self, point, rng, callback, **kwargs):

    """

    Skyspark will not accept GET request for his_read by default

    [ref : https://project-haystack.org/forum/topic/787#c6]

        The default behavior of SkySpark is now to disallow GET requests

        non-idempotent operations. So its still allowed on certain operations

        such as about, formats, read. However as Chris said it can be toggled

        back on using Settings|API for backward compatibility.

        However as a recommendation I think we should always be using POST as

        a safer alternative. Using GET for ops with side-effects is against

        the HTTP spec. Plus it is an attack vector if cookies are involved.

        And it provides a more precise way to pass the request payload.

        Its not really from a theoretical perspective. But in SkySpark

        we allow customers to generate histories using their own custom

        functions. So from a security perspective we took the safest route

        and consider it to potentially have side effects.

        If your code is all using GET, then just have the customer set

        Settings|API allowGetWithSideEffects flag to false and it should all work.

    """

    if isinstance(rng, slice):

        str_rng = ",".join([hszinc.dump_scalar(p) for p in (rng.start, rng.stop)])

    elif not isinstance(rng, string_types):

        str_rng = hszinc.dump_scalar(rng)

    else:

        # Better be valid!

        str_rng = rng

    col_list = [('id',[]),('range',[])]

    his_grid = hszinc.grid.Grid(columns=col_list)

    his_grid.insert(0,{"id": self._obj_to_ref(point), "range": str_rng})

    print(his_grid)

    return self._post_grid(

        "hisRead",

        his_grid,

        callback,

        **kwargs

    )

I think I got it right (not sure my Grid is number 1 but it seem to work) ... but server throws :

HTTPError: 400 Client Error: SkyArc-Attest-Key required for url: https://demo.skyfoundry.com/api/demo/hisRead

This confirms what @clarsen https://github.com/clarsen was seing in issue #84 https://github.com/ChristianTremblay/pyhaystack/issues/84

Reading through forums and doc, i see this

If you are not using a browser, then don't use cookies! Use a bearer token with authorization header

Then my knowledge is not strong enough to overpass this. I don't get it.

@sjlongland https://github.com/sjlongland if you can bring some light it would be awesome.

I'm also poking people that took part of issue #84 https://github.com/ChristianTremblay/pyhaystack/issues/84

@clarsen https://github.com/clarsen you were near something in april, maybe you can help

@gmanohar25 https://github.com/gmanohar25

@San42 https://github.com/San42

@justindarcy https://github.com/justindarcy

@dawcurious https://github.com/dawcurious

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/ChristianTremblay/pyhaystack/issues/91#issuecomment-678723743, or unsubscribe https://github.com/notifications/unsubscribe-auth/AAALZFP2ZCPP5GLZUY5DO6TSCCDB7ANCNFSM4QGL7UUQ .

[sjlongland]

Okay, so the cookie stuff… that could be tricky as I tried very hard to round-trip cookies set by the server, and now they want us to do the opposite?

[ChristianTremblay]

@clarsen Submitted where ? I missed something ?

[sjlongland]

https://github.com/ChristianTremblay/pyhaystack/blob/master/pyhaystack/client/http/base.py#L160-L163

Okay, so we need to set exclude_cookies=True in requests made to SkySpark.

[sjlongland]

Okay, so python-requests is too clever for its own good I think… it was ignoring me when told to not round-trip cookies.

https://github.com/sjlongland/pyhaystack/tree/skyspark-his-read-fix

I actually have converted all GET requests to POSTs, things seem to work. I don't see it being a problem for the other implementations either. We should give that a shot and see what happens.

[clarsen]

@clarsen Submitted where ? I missed something ? Sorry i only shared 1/2 of the code involved. https://github.com/ChristianTremblay/pyhaystack/pull/93 has the modifications to strip out cookies.

[ChristianTremblay]

@sjlongland and I worked really hard and we finally found something that works the way we wanted.

Develop branch have just been updated and all the tests I have made are successful with nhaystack and Skyspark.

For now, only his_read is treated as POST. Next step will be to convert write to POST... but for now, let's celebrate his_read working on skyspark.

@clarsen thanks for your contribution

This will be merge to master ASAP so PyPI version works for Skyspark. So please give any feedback.

Thanks

[sjlongland]

@ChristianTremblay hisWrite should be a POST request already… as it's often large blocks of data being written, more than what would fit in a GET request. :-)

[ChristianTremblay]

I was thinking about point_write

[ChristianTremblay]

CLosing this as this is working in the develop branch. New release soon