Skip to content

Conversation

@EloB
Copy link

@EloB EloB commented Jan 24, 2023

What does it do?

I've added possibility to change fields, schema and grant configs. Also added a example plugin with a new Auth provider for Apple in ./examples/getstarted.

Created separate commits to easily overview what I've done.

These commits can be removed if we don't want that Apple plugin in the codebase. I've added it there for testing purposes. I will put that into a separate plugin on NPM if this PR will be merged.
759f4600b9097ab547c604e4e6ecb8ae10647f0f
0f6a335245e7d0d4b8bb87a700e17a8fbe6397fb

With these changes it will be able to create isolated auth provider plugins with custom fields etc.
image

Why is it needed?

Right now it's not possible to support more advanced Auth providers like Apple OAuth without overriding files in users-permissions plugin.

How to test it?

This is just a draft. If the team likes the PR than we can discuss how to test it.

Related issue(s)/PR(s)

I've created this PR and it was closed. This PR is taking an alternative route by changes to the core so I can create a plugin that contains Apple Provider and than publish that to the marketplace and maintain that myself.
#15522

@EloB EloB changed the title Feature: ability to add custom auth providers Feature: ability to add custom auth providers in plugin Jan 24, 2023
@EloB EloB changed the title Feature: ability to add custom auth providers in plugin Feature: ability to add custom auth providers in plugins Jan 24, 2023
@EloB EloB changed the title Feature: ability to add custom auth providers in plugins Feature: ability to add custom auth providers in plugin Jan 24, 2023
Comment on lines +5 to +46
module.exports = ({ strapi }) => {
strapi.service('plugin::users-permissions.providers-registry').register(
'apple',
({ purest }) => {
const apple = async ({ accessToken }) => {
const tokenPayload = jwt.decode(accessToken);
if (!tokenPayload) {
throw new Error('unable to decode jwt token');
}
return {
username: tokenPayload.sub,
email: tokenPayload.email,
};
};

return apple;
},
{
defaultGrantConfig: (baseURL) => ({
enabled: false,
icon: 'apple',
key: '',
secret: '',
teamId: '',
keyIdentifier: '',
callback: `${baseURL}/apple/callback`,
scope: ['name', 'email'],
nonce: true,
state: true,
custom_params: {
response_type: 'code id_token',
response_mode: 'form_post',
},
}),
buildRedirectUri: ({ urlJoin, baseURL, apiPrefix }) => urlJoin(baseURL, 'apple', 'callback'),
connectGrant: (config) => ({
...config,
secret: strapi.service('plugin::apple.auth').createSecret(config),
}),
}
);
};
Copy link
Author

@EloB EloB Jan 24, 2023

Choose a reason for hiding this comment

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

To create a plugin with a Auth provider you use the register lifecycle and register a new provider.

Comment on lines +18 to +87
register(app) {
const plugin = {
id: pluginId,
initializer: Initializer,
isReady: false,
name,
};

app.registerPlugin(plugin);

app.appPlugins['users-permissions'].transformFields('apple', (layout) => {
const BEFORE_CALLBACK = layout.form.findIndex((items) =>
items.find(({ name }) => name === 'callback')
);

return {
...layout,
form: insertFields(
replaceFields(layout.form, {
secret: {
type: 'textarea',
},
}),
BEFORE_CALLBACK,
[
{
intlLabel: {
id: 'todo.teamId',
defaultMessage: 'Team ID',
},
name: 'teamId',
type: 'text',
placeholder: {
id: 'todo.placeholder',
defaultMessage: 'TEXT',
},
size: 12,
validations: {
required: true,
},
},
],
[
{
intlLabel: {
id: 'todo.keyIdentifier',
defaultMessage: 'Key ID',
},
name: 'keyIdentifier',
type: 'text',
placeholder: {
id: 'todo.placeholder',
defaultMessage: 'TEXT',
},
size: 12,
validations: {
required: true,
},
},
]
),
};
});
app.registerPlugin({
id: pluginId,
initializer: Initializer,
isReady: false,
name,
});
},
Copy link
Author

Choose a reason for hiding this comment

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

Inside your plugin admin section you can now also register transformFields to make changes to that Modal form. Important that you do those changes immutable.

Copy link
Author

Choose a reason for hiding this comment

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

This also support changing the schema but this example is not using it.

@derrickmehaffy
Copy link
Member

I do like this idea as a temporary way to get around the slightly more complex extension process (until we refactor/rebuild U&P and move away from Grant to probably passport instead)

@codecov
Copy link

codecov bot commented Jan 24, 2023

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 50.62%. Comparing base (7ab3dc3) to head (0f6a335).
Report is 7289 commits behind head on main.

Additional details and impacted files
@@             Coverage Diff             @@
##             main   #15559       +/-   ##
===========================================
- Coverage   66.11%   50.62%   -15.50%     
===========================================
  Files        1056      297      -759     
  Lines       23017    10474    -12543     
  Branches     4129     2324     -1805     
===========================================
- Hits        15218     5302     -9916     
+ Misses       6872     4272     -2600     
+ Partials      927      900       -27     
Flag Coverage Δ
back 50.62% <ø> (?)
front ?
unit_back 50.62% <ø> (?)
unit_front ?

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@joshuaellis joshuaellis requested a review from Marc-Roig January 25, 2023 08:13
@Marc-Roig Marc-Roig removed their request for review January 25, 2023 10:47
@Marc-Roig
Copy link
Contributor

Awesome initiative :) We will discuss this internally and assign someone to review the PR , in the meantime, can you take a look at the tests?

@EloB
Copy link
Author

EloB commented Jan 25, 2023

@Marc-Roig thanks mate! I can take a look at that. Wasn't sure if you guys were going to like it. So didn’t spend so much time on it…

Trying to get familiar with the codebase. :)

@Boegie19
Copy link
Collaborator

@EloB If you have any questions about anything or want help with anything best place to get a quick answer is the discord in strapis readme where we also have open office hours every work day with strapi employs every Mon - Fri 12:30 CST

@EloB
Copy link
Author

EloB commented Jan 25, 2023

@Boegie19 I’ve joined Discord. Will try to join a call on Discord 🙂

@EloB
Copy link
Author

EloB commented Jan 25, 2023

@derrickmehaffy I also prefer Passport over Grant and totally agree that this is only a temporary solution but yet needed one until someone have time to refactor the Auth beast :D.

@derrickmehaffy
Copy link
Member

@derrickmehaffy I also prefer Passport over Grant and totally agree that this is only a temporary solution but yet needed one until someone have time to refactor the Auth beast :D.

Yeah we still aren't 100% sure what the plan is but the two proposed options were:

  • refactor u&p from the ground up when we work on Strapi v5 at the end of 2023-ish
  • build an entirely new plugin and slowly phase out u&p (but both would exist in parallel for a time)

We are probably leaning towards option 2 so in that we would make sure to use passport instead and build a provider system much like our email and upload plugins use.

@strapi-bot
Copy link

This pull request has been mentioned on Strapi Community Forum. There might be relevant details there:

https://forum.strapi.io/t/is-social-auth-still-feasible-in-strapi/25265/2

@ronronscelestes ronronscelestes added source: plugin:users-permissions Source is plugin/users-permissions package pr: feature This PR adds a new feature labels Jan 30, 2023
@Marc-Roig
Copy link
Contributor

Marc-Roig commented Feb 15, 2023

Hello @EloB , this work is awesome and we have been reviewing both this PR and #15522. Congrats on doing this.

We have a couple of thoughts, and as Derrick said in the future we might build a new plugin entirely. But this could be a quick win in the meantime, to allow people to use custom providers.

Revising the PR, we think we should take over the work and we want to follow this direction:

  • Instead of building a plugin for your provider, it could be done with the provider pattern. The same as in 'upload', where you have the cloudinary, s3-bucket,... providers, we could have the same here.

This would allow a more clear configuration:

// /plugins.js
module.exports = () => {
	'users-permissions': {
		config: {
			provider: 'apple',
			providerOptions: ...
		}
	}
}

By looking at what's necessary , the provider should incorporate:

  • A way to extend the grantConfig
    • A method where you can generate secrets as the createAppleSecret in your PR
  • A way to extend the provider-registry handler
    • And ensure it always return username and email

Also, as we do not have keys for the apple provider, we could not test this PR :) But we will use a similar structure as yours for implementing this.

We will make a formal proposal for this and start working on it if everyone agrees :)

@alexandrebodin
Copy link
Member

Hi @EloB, thank you for these contributions !

I think if we want to move faster we would benefit from making this simpler.

What would you think about adding & configuring the providers on the config side only ? This would sadly require redeploys so that's a treadoff but I'm not sure managing these kind of config from the admin was a good thing to start with :)

@petersg83 petersg83 removed their assignment Jun 20, 2023
@Boegie19
Copy link
Collaborator

Any updates on this?

@joshuaellis joshuaellis removed the request for review from petersg83 June 27, 2023 16:28
@hanpaine hanpaine removed the request for review from Marc-Roig March 21, 2024 11:03
@alexandrebodin
Copy link
Member

alexandrebodin commented Jun 26, 2024

Hi, I'm going to close this PR in favor of #20634. This for now will only be introduced in v5.
Feel free to try it out in the upcoming v5 betas/release candidates and share feedbacks

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

pr: feature This PR adds a new feature source: plugin:users-permissions Source is plugin/users-permissions package

Projects

Status: Fixed/Shipped

Development

Successfully merging this pull request may close these issues.

8 participants