Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Re-introduce "Fix valid signature" from OSPR-1539 #15352

Closed
wants to merge 1 commit into from

Conversation

@schenedx
Copy link
Member

schenedx commented Jun 19, 2017

This reverts commit 4ce1222.

Because we reverted this change for LEARNER-1464, I would like to introduce this change back.
I will be talking with Software Secure on this signature change and only will merge after we developed a plan to integrate this change with SoftwareSecure.

This reverts commit 4ce1222.
@schenedx schenedx requested review from gsong and andy-armstrong Jun 19, 2017
@schenedx schenedx mentioned this pull request Jun 19, 2017
0 of 1 task complete
Copy link
Contributor

andy-armstrong left a comment

👍 thanks for resurrecting this. I'll be interested to hear how Software Secure recommends handling this change.

@gsong
gsong approved these changes Jul 24, 2017
@schenedx

This comment has been minimized.

Copy link
Member Author

schenedx commented Jul 24, 2017

@gsong @andy-armstrong
After I discussed with SoftwareSecure, I believe this change should not be merged at it's current state.

See my inline comment

@@ -887,8 +887,10 @@ def create_request(self, copy_id_photo_from=None):
"Content-Type": "application/json",
"Date": formatdate(timeval=None, localtime=False, usegmt=True)
}

body_for_signature = {"EdX-ID": str(self.receipt_id)}

This comment has been minimized.

Copy link
@schenedx

schenedx Jul 24, 2017

Author Member

The comment from Software Secure is:

This block of code is generating a signature that does not match the content that is being returned as the body. In other words, the authorization signature they're generating to put in the header is one for a request body that is only the EdX-ID, whereas the body they're actually sending is that bigger chunk above defined in body = {}. So our end is rejecting it, as the body the API is receiving and signing inevitably ends up not matching the one they provide, as they signed different content than we did.

@schenedx

This comment has been minimized.

Copy link
Member Author

schenedx commented Oct 25, 2018

We are not making a change like this here.

@schenedx schenedx closed this Oct 25, 2018
@edx-webhook

This comment has been minimized.

Copy link

edx-webhook commented Oct 25, 2018

@schenedx Even though your pull request wasn’t merged, please take a moment to answer a two question survey so we can improve your experience in the future.

@schenedx schenedx deleted the schen/OSPR-1539 branch Oct 25, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
4 participants
You can’t perform that action at this time.