Skip to content
This repository has been archived by the owner on Oct 22, 2021. It is now read-only.

Add Oauth2 Support And Add Path Support in sdk #12

Closed
wants to merge 6 commits into from

Conversation

zxy-lgtm
Copy link
Member

@zxy-lgtm zxy-lgtm commented Aug 13, 2021

there also have some problems :
I use code flow to get a oauth2 access_token so that we can refresh it to enable long-term use of the API in some scenarios.
But in the first step, I can only access get on the page_ code_ urlmethod to obtain the corresponding'codein the redirection. Thecodeis attached to the redirection URL set by Azure. I think you can obtain the code by obtaining the redirected URL, but it is not successful.
https://docs.microsoft.com/en-us/onedrive/developer/rest-api/getting-started/msa-oauth?view=odsp-graph-online

@xxchan
Copy link

xxchan commented Aug 14, 2021

Hi, about oauth2, we can let the user decide how to generate the token and simply pass it as the comment says and the PR does. So utils_token is not needed (but you can use it to test for yourself).

(utils_drives.go not yet reviewed

BTW, A suggestions: Use descriptive PR title (what problem you solves). In the PR description, you can say closes #11 (or ref #11 if the PR only resolves part of the issue).

utils_token.go Outdated
Comment on lines 28 to 32
const (
secret = "-uGEL0oQZ86Wwr2I-dTF3~Y3i6sCGE5z~n"
client_id = "8ce1780f-12cc-45eb-a4c9-83f284e56621"
redirect_url = "http://localhost:9999"
)
Copy link

@xxchan xxchan Aug 14, 2021

Choose a reason for hiding this comment

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

Generally these sensitive data should not be pushed to GitHub. (You can use GitHub secrets when necessary.) And it seems that we don't need utils_token as #12 (comment) says

Copy link

Choose a reason for hiding this comment

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

You can try to remove this file from commit history... (a chance to learn advanced git operation 🤣

@zxy-lgtm zxy-lgtm changed the title #issue 11 Add Oauth2 Support And Path Support in sdk Aug 14, 2021
@zxy-lgtm zxy-lgtm changed the title Add Oauth2 Support And Path Support in sdk Add Oauth2 Support And Add Path Support in sdk Aug 14, 2021
@junaire
Copy link
Member

junaire commented Aug 14, 2021

Hello, @zxy-lgtm, looks like you are in the trouble. Maybe I can give you some suggestions:

  • We prefer create new branches every time open a PR, see beyondstorage/go-storage/pull/697#issuecomment
  • Read code about go-storage and go-service-*. I highly recommend you to read go-service-gdrive and go-service-s3. You can find that many codes are very similar or even duplicated. So implement go-service-* is not particularly complicated.
  • Read code about onedrive. I just did a simple search, you can read koofr/go-onedriveclient, or rclone, they are really good examples.
  • Don't hesitate to ask for help if you are in trouble. You can reach us on github issues, telegram, or matrix. Everyone in the commnity is happy to help you.

At last, happy hacking! ;-)

zxy-lgtm and others added 2 commits August 14, 2021 23:10
change it into utils_token.go
@xxchan
Copy link

xxchan commented Aug 18, 2021

What about pushing Storager implementations together, instead of only the utils? (i.e., how do you use these utils)

@Xuanwo
Copy link
Contributor

Xuanwo commented Aug 20, 2021

Hello, any updates here?

@xxchan
Copy link

xxchan commented Sep 6, 2021

Hello, any updates? (Can you tell us your schedule?)

@zxy-lgtm
Copy link
Member Author

zxy-lgtm commented Sep 6, 2021 via email

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants