Skip to content
This repository has been archived by the owner on Jan 14, 2020. It is now read-only.

Flask RESTful #28

Merged
merged 73 commits into from
Apr 30, 2018
Merged

Flask RESTful #28

merged 73 commits into from
Apr 30, 2018

Conversation

tyleryasaka
Copy link
Contributor

@tyleryasaka tyleryasaka commented Apr 25, 2018 •
edited
Loading

Checklist:

  • Code contains relevant tests for the problem you are solving
  • Ensure all new and existing tests pass
  • Update any relevant READMEs and docs
  • Submit to the develop branch instead of master

Description:

Contains commits from #27 (would be easier to review/merge that first)

  • Swaps out apilib in exchange for Flask-RESTful and Marshmallow (And yes, I did try the flask-marshmallow library. It doesn't provide the serialization and deserialization that we need.)
  • Cleans up API interface to be as standard and implementation-agnostic as possible; this API should be able to be replicated in other frameworks (e.g. RoR, Node/Express).
  • Creates documentation for the Attestations API.
  • Have this code up and running on heroku if anyone wants to test it out: http://bridge-server-test.herokuapp.com/

Finishes #21, #22, #8, #9, #7, #3, #23

Tyler Yasaka and others added 30 commits April 23, 2018 16:56
@tyleryasaka
Install web3
019ed68
@tyleryasaka
Use valid ethereum addresses in tests
7c7fc9f
@tyleryasaka
Return actual signed attestation message
53e7719
@tyleryasaka
Test that correct attestation is returned
a769316
@tyleryasaka
Return attestation as 3 separate fields
c4d4c8f 
Signature, claim type, and data.
@tyleryasaka
Move generate_signature method to a new util for reuse
ab657a3
@tyleryasaka
Correctly order arguments in equality assertions
71b31fb
@tyleryasaka
Fix readme typo
1126bec
@tyleryasaka
Install sendgrid
11f1e2c
@tyleryasaka
Add email column to VerificationCode
49a081c
@tyleryasaka
Add email env vars
dc1ad28
@tyleryasaka
Add email verification API endpoints
71193a1
@tyleryasaka
Implement email verification
f07c1b1
@tyleryasaka
Match email/phone along with eth address to find existing code
cdd268d
@tyleryasaka
Test email verification methods
2bed1fc
@tyleryasaka
Add endpoint to get facebook auth url
5a068d5
@tyleryasaka
Test get_facebook_auth_url
d922ed1
@tyleryasaka
Rename get_facebook_auth_url -> facebook_auth_url
0c0cc11
@tyleryasaka
Add endpoint for facebook verification
73eea0b
@tyleryasaka
Test facebook verification
e37433e
@tyleryasaka
Make request/response class names consistent
43318b6
@tyleryasaka
Install Flask-Session and oauth2
2c2eed1
@tyleryasaka
Set up session in app
af9c79f
@tyleryasaka
Add twitter env vars
bff7071
@tyleryasaka
Add twitter auth url endpoint
56c18fc 
Returns a url that the user can navigate to to authorize Twitter
@tyleryasaka
Add twitter verification endpoint
bf69b67 
Returns an attestation if verification is successful
@tyleryasaka
Ignore files in flask_session
71d71c9 
Currently Flask Session is using the filesystem to store session data.
Files are placed in `flask_session`. Eventually we probably want to
switch to Redis or something, but this should do for now.
@tyleryasaka
Instantiate client inside method
3fcc8e5
@tyleryasaka
Fix deprecation warning
5933ec2
@tyleryasaka
Test twitter verification
233d4d1
@tyleryasaka tyleryasaka requested a review from Gzing April 25, 2018 22:55
@tyleryasaka tyleryasaka mentioned this pull request Apr 26, 2018
Closed
4 tasks
crazybuster and others added 5 commits April 26, 2018 16:10
@crazybuster
New end to end integration testsfor phone and email
148b90e
@tyleryasaka
Merge branch 'correct_db_params' of github.com:Gzing/bridge-server in…
152dca4 
…to HEAD
@tyleryasaka
Add integration test for verifying twitter & facebook
6a96ab9
@tyleryasaka
Test for signature instead of data in integration test
890668b 
If we're only going to check for one thing, it should be the signature. Technically the other pieces of data should be known by the client beforehand, and are not strictly needed in the response.
@tyleryasaka
Handle case where session does not exist
bad672b 
Before this would throw a key error; now we're returning an error message.
@Gzing
Copy link

Gzing commented Apr 27, 2018

@tyleryasaka is this still in WIP or should I test/review this and try it out on my local?

@tyleryasaka
Copy link
Contributor Author

@Gzing It's ready for review!

Gzing
Gzing approved these changes Apr 27, 2018
View reviewed changes
Copy link

@Gzing Gzing left a comment •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM
Really digging the restful + marshmallow API setup. 👍

We could add max wrong tries on Phone and Email verification endpoints to make those more secure. Will add that as a separate issue.

crazybuster
crazybuster approved these changes Apr 30, 2018
View reviewed changes
Copy link
Contributor

@crazybuster crazybuster left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good to me.

NOTE: Added an issue for more logic centric exceptions with issue#29, also we should probably change the return types from dicts to better typed return objects.

@crazybuster crazybuster merged commit 5131b3b into develop Apr 30, 2018
@crazybuster crazybuster deleted the flask-restful branch April 30, 2018 17:41
This was referenced May 1, 2018
Closed
Closed
Closed
Closed
Closed
Closed
Closed
Closed
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Reviewers

@Gzing Gzing Gzing approved these changes

@crazybuster crazybuster crazybuster approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
3 participants
@tyleryasaka @Gzing @crazybuster