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

Feature/expand account authentication #100

Merged
merged 6 commits into from
May 5, 2021

Conversation

corowne
Copy link
Owner

@corowne corowne commented May 3, 2021

Authentication of various other social media sites

  • Ability to choose the initial site linked from the primary ones available in the sites config
  • Aliases management in user settings
  • Change primary alias, hide non-primary aliases, remove non-primary aliases
  • By default it's possible to link multiple accounts on the same site (but not the same account multiple times)
  • List of aliases linked in sidebar on user profile page - non-paginated for now as it seems unlikely to become a very long page

Misc

  • Changed fullName in sites config to full_name
  • Slightly adjusted user admin panel page visually to handle page length better

.env must be updated to contain:

DEVIANTART_CLIENT_ID=
DEVIANTART_CLIENT_SECRET=

TWITTER_CLIENT_ID=
TWITTER_CLIENT_SECRET=

INSTAGRAM_CLIENT_ID=
INSTAGRAM_CLIENT_SECRET=

TUMBLR_CLIENT_ID=
TUMBLR_CLIENT_SECRET=

IMGUR_CLIENT_ID=
IMGUR_CLIENT_SECRET=

TWITCH_CLIENT_ID=
TWITCH_CLIENT_SECRET=

as required. It should be safe to leave out providers that will not be used for authentication.
Currently putting the redirect URIs into the services.php file - if this causes any issues (not seeing any on local testing), they can be added into .env as an additional {PROVIDER}_REDIRECT_URI field, like so:
DEVIANTART_REDIRECT_URI=/auth/callback/deviantart

- Ability to choose the initial site linked from the primary ones available in the sites config
- Aliases management in user settings
- Change primary alias, hide non-primary aliases, remove non-primary aliases
- By default it's possible to link multiple accounts on the same site (but not the same account multiple times)
- List of aliases linked in sidebar on user profile page - non-paginated for now as it seems unlikely to become a very long page

Misc
- Changed fullName in sites config to full_name
- Slightly adjusted user admin panel page visually to handle page length better

.env must be updated to contain:

DEVIANTART_CLIENT_ID=
DEVIANTART_CLIENT_SECRET=

TWITTER_CLIENT_ID=
TWITTER_CLIENT_SECRET=

INSTAGRAM_CLIENT_ID=
INSTAGRAM_CLIENT_SECRET=

TUMBLR_CLIENT_ID=
TUMBLR_CLIENT_SECRET=

IMGUR_CLIENT_ID=
IMGUR_CLIENT_SECRET=

TWITCH_CLIENT_ID=
TWITCH_CLIENT_SECRET=

as required. It should be safe to leave out providers that will not be used for authentication.
Currently putting the redirect URIs into the services.php file - if this causes any issues (not seeing any on local testing), they can be added into .env as an additional {PROVIDER}_REDIRECT_URI field, like so:
DEVIANTART_REDIRECT_URI=/auth/callback/deviantart
@itinerare itinerare added the needs review Pull requests that are pending community review label May 3, 2021
@itinerare itinerare added this to the v2.0.0 milestone May 3, 2021
@itinerare itinerare added the enhancement New feature or request label May 3, 2021
Copy link
Collaborator

@itinerare itinerare left a comment

Choose a reason for hiding this comment

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

Adjusted review based on bug report/confirmation that it occurred on a clean copy-- looks good to me besides for this, however

@@ -52,7 +69,7 @@ public function user()
public function getUrlAttribute()
{
if($this->site == 'tumblr') return 'https://'.$this->alias.Config::get('lorekeeper.sites.tumblr.link');
else return 'https://'.Config::get('lorekeeper.sites.'.$this->site.'.link').'/'.$this->alias;
else return 'https://'.Config::get('lorekeeper.sites.'.$this->siteDisplayName.'.link').'/'.$this->alias;
Copy link
Collaborator

@itinerare itinerare May 4, 2021

Choose a reason for hiding this comment

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

Needs to be changed back to $this->site I believe; this is consistent with the command to adjust dA aliases already on file

Copy link
Collaborator

@Draginraptor Draginraptor left a comment

Choose a reason for hiding this comment

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

Besides the $this->site issue mentioned, the rest of the code looks fine

@corowne corowne requested a review from itinerare May 5, 2021 15:35
@itinerare itinerare added reviewed Pull requests that have received community review and are pending merge and removed needs review Pull requests that are pending community review labels May 5, 2021
@itinerare itinerare merged commit dfbbdd9 into develop May 5, 2021
@itinerare itinerare deleted the feature/expand-account-authentication branch May 5, 2021 15:41
@itinerare itinerare mentioned this pull request May 5, 2021
28 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request reviewed Pull requests that have received community review and are pending merge
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants