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

Add support for OAuth2 authentication, source property on Events #163

Merged
merged 12 commits into from
Aug 22, 2020

Conversation

akmolina28
Copy link
Contributor

@akmolina28 akmolina28 commented Aug 6, 2020

Thanks for this awesome project! I needed some new features and I thought they were worth sharing with the community.

New features:

  • Authenticate using OAuth2 as an alternative to using service accounts
  • Add getter/setter for the source property on Events (also added a test for this)

Background:

Capture

The source property on the google event object allows you to set a custom url as an attribute in the calendar event. The link is visible and clickable in the Google Calendar interface. While the source property was accessible via $event->googleEvent->getSource() and $event->googleEvent->setSource(), it was not working when authenticating with a service account.

A closer look at the api documentation reveals the problem:

Source from which the event was created. For example, a web page, an email message or any document identifiable by an URL with HTTP or HTTPS scheme. Can only be seen or modified by the creator of the event.

This means that only the user who created the event can see the link, i.e. the link is only visible to the service account user. This is why I forked to add support for authenticating using my own Google account.

Implementation:

The new authentication method can be configured by setting a new environment variable, GOOGLE_CALENDAR_AUTH_PROFILE. Setting this variable to oauth will select the OAuth2 profile. The config will default to using the service account if you do not set anything. The code will switch to authenticate the client based on the selected profile. I have included instructions for obtaining the OAuth2 credentials in the README.

The new getter/setter for the Event source property can be used no matter how you authenticate, but it will only be readable by the creator of the event.

Upgrading:

Due to changes in the config file, if you upgrade from an older version you will have to publish a fresh config. I have included a note about this in the readme.

Copy link
Member

@freekmurze freekmurze left a comment

Choose a reason for hiding this comment

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

Very nice, though I have a few nitpicks.

I can imagine it's hard to write a test for the OAuth stuff. Did you manually test this and are a 100% sure that it works?

Due to the changes in the config file, this is indeed a breaking change, but I'm willing to tag a new major version for this feature.

src/GoogleCalendarServiceProvider.php Outdated Show resolved Hide resolved
src/Event.php Outdated Show resolved Hide resolved
src/GoogleCalendarFactory.php Outdated Show resolved Hide resolved
src/GoogleCalendarFactory.php Outdated Show resolved Hide resolved
src/GoogleCalendarFactory.php Show resolved Hide resolved
@akmolina28
Copy link
Contributor Author

Thanks for the feedback. With respect to testing, I have been using my fork in a project I am working on and I have a few tests that create/update/delete actual calendar events. I have been running those tests to make sure that both OAuth and Service Account authentication work. OAuth is working perfectly.

I would consider refactoring to make this a non-breaking change, but I liked the idea of having different profiles, similar to how laravel lets you select different database types. This made it really easy to test both auth methods by just toggling an environment variable, and it sets up some very clear guard rails for someone to add more profiles in the future.

@akmolina28 akmolina28 requested a review from freekmurze August 8, 2020 17:00
@freekmurze freekmurze merged commit ea4faf2 into spatie:master Aug 22, 2020
@freekmurze
Copy link
Member

Thanks!

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

Successfully merging this pull request may close these issues.

2 participants