-
Notifications
You must be signed in to change notification settings - Fork 0
Add external api #54
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
base: main
Are you sure you want to change the base?
Add external api #54
Conversation
| 'peerId': peerId, | ||
| 'orgName': orgName, | ||
| }); | ||
| var response = await http.post( |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
we might need a try catch
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ok will do that.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
http post can fail too, we need catch for that case
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ok.
| } | ||
|
|
||
| class OrgAccessToken { | ||
| static final int _accessTokenMaxAge = 30; // days |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
token age might be something we have to worry about but not during a poc. We need to come back on this.
| return Response.ok('Transaction request received'); | ||
| } | ||
|
|
||
| Future<bool> externalApi({required String did,required String peerId,required String orgName,required String callBackUrl,required Token orgAccessToken })async { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This function should be named fireAuthCallback or something similar. It
doesn't need orgName
did and peerId need not be separate
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ok will make these changes and put a new commit.
|
Also generate protos again to fix the conflicts, I had merged a pending PR to avoid confusion |
|
|
||
| Future<bool> externalApi({required String did,required String peerId,required String orgName,required String callBackUrl,required Token orgAccessToken })async { | ||
| var bodyJsonStr = jsonEncode({ | ||
| 'did': did, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
session id too
| Uri.http(callBackUrl), | ||
| headers: <String, String>{ | ||
| 'Content-Type': 'application/json; charset=UTF-8', | ||
| 'Authorization': 'Bearer $orgAccessToken', |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
token should move to body. We are giving a token for them to access our apis
| Future<bool> externalApi({required String did,required String peerId,required String orgName,required String callBackUrl,required Token orgAccessToken })async { | ||
| var bodyJsonStr = jsonEncode({ | ||
| 'did': did, | ||
| 'peerId': peerId, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
add chain too. also add chain in access token
| return Response.ok('Transaction request received'); | ||
| } | ||
|
|
||
| Future<bool> externalApi({required String did,required String peerId,required String orgName,required String callBackUrl,required Token orgAccessToken })async { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
at the last, this function shall move to another file as it is generic. Shall create a new module, - sky_outgoing_calls
a68e34c to
f56cb65
Compare
| return false; | ||
| } | ||
| }catch(e){ | ||
| print(e); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
print stack trace too
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Sure
85a87f0 to
03a49ac
Compare
03a49ac to
dbdf580
Compare
| }); | ||
| try{ | ||
| var response = await http.post( | ||
| Uri.http(callBackUrl), |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Use Uri.parse
| var bodyJsonStr = jsonEncode({ | ||
| 'did': '$did.$peerId', | ||
| 'session_id': sessionId, | ||
| 'token': orgAccessToken, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
orgAccessToken is of Token class, we just need the token value inside it which is a string, that is what we can pass through an api.
| Future<OrgStatus> approveOrgAuthRequest( | ||
| ServiceCall call, OrgAuthRequest request) async { | ||
| var user = RubixService.getAuthUser(call); | ||
| const rubix = 'rubix'; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
this must have been coming from phone,
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
no, no, not need as this is rubix ext rpc. But could have simply passed this to the next function without having to store it in a variable.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ok will make that change.
38c1334 to
350c94f
Compare
No description provided.