You want to authenticate a 3rd party website using OAuth2? What do you mean by on the backend? The authentication process itself to obtain a token or the usage of the tokens in a request triggered by the backend?
Hmmm… Not I don’t want to authenticate a 3rd party website using OAuth2.
I give you the context.
I work on the Workspace app and it uses the Groupfolders app.
You can see Workspace app as a plugin for the Groupfolders app.
With @StCyr , we interacted with Groupfolders via its API/REST the back-end side.
But, we have encountered problems to use those API/REST with SSO users.
So, to resolve this problem, we migrated all API/REST calls from Groupfolders to the front-end.
Unfortunaely, I doubt that OAuth2 will help you much here. The NC implementation only allows to authenticate users: third party apps can request and check the validity of a user by means of the question if this user is part of a NC instance. Similar to the common login via google/github/facebook/… on many pages. AFAIK the implementation does not provide authentication, so you cannot use these to login in NC itself. There is another app to provide this but this is a separate issue/topic.
Regarding the dependency injection issue:
You should be able to
use OCA\GroupFolders\Folder\FolderManager
This is independent of the class is present or not. Then you can do
if (class_exists(FolderManager::class)) { /* ... */ }
You must not use the class unless you know it is reachable by PHP. So you must not simply write a class with the appropriate constructor but you will ost probably have to write the same class twice (like a wrapper) and throw an exception in the case the class was not found. Here is an untested code snippet to get the idea:
if (class_exists(FolderManager::class) {
class FolderManagerWrapper {
public function __construct(FolderManager(FolderManager $fm) {
$this->fm = $fm;
}
public function getManager() {
return $this->fm;
}
}
} else {
class FolderManagerWrapper {
public function __construct() {
}
public function getManager() {
throw new Exception('The Groupfolder app was not installed');
}
}
}
As I said, I have not tested it yet. So you might need to tweak a bit.
Is this what you where looking for?
That’s what I thought… It’s a really important question to know how to integrate apps into other apps.
Oooh nice topic
I never seen it until now !
I’m glad to hear about it. I thought I was alone on this problem ><
It’s okay, I think I understand your idea. You advise me to create a wrapper and not use FolderManager directly…
Hmmm… I will see the code tomorrow, but the idea is this :
<?php
class FolderManagerWrapper() {
private static $instance = null;
private function __construct(private FolderManager $fm) {
}
public function getInstance(): FolderManagerWrapper {
if (!class_exists(FolderManager::class)) {
throw new Exception("The FolderManager class does not exist.");
}
if (is_null(self::$instance) {
self::instance = new FolderManagerWrapper() // I should inject its dependencies here
}
return self::$instance;
}
}
It’s a singleton, so I will test it tomorrow. It’s the way I see it.
But, in your idea, I use the FolderManager “directly”. However, it’s not a best practice for Nextcloud.
I told about it with Christoph Wurst on the Collective app : Nextcloud .
The collective app depends directly Circles, but I don’t understand why we have apps that make exceptions
All that to say, if I use FolderManager, even as a wrapper, am I in the same case than the Collective App ?
Do I respect the Nextcloud’s rules ?
Edit - 12/07/2023 at 11.57pm
I tested this code and it works !
<?php
$isActive = false;
if ($isActive) {
class A {
}
} else {
die("Error is not active");
}
$a = new A();
var_dump($a);
This is not 100% going to work. The problem is that your service class will have a dependency injection on the class (A in the case). This will work if the class is to be found ($isActive == true`) but fail otherwise.
The idea was to provide two class implementations, one with the wrapper functional and one with a default implementation that does not depend on third party app code. With the check you select which implementation should be used. The default implementation will/must therefore not depend on the third party class in any way (dependency injection in the constructor or any other use case).
You could also do a similar pattern as the server where you define an interface (IA) and multiple classes that implement the interface (AWrapped and ADefault). Both classes AWrapped and ADefault can be implemented as usual to use the foreign class A (AWrapped) or throw a default exception. You can then in your Application class register either class as an implementation of IA depending on the existence of class A (see the docs on the manual dependency injection).
Yeah, accessing foreign classes is brittle and tends to be more work. Also you might need to keep a list of compatible version combinations of the involved apps. Not nice.
The thing is that you would need support by the other app in order to do other things. I personally think the event system to be a good way but then the groupfolder app needs to provide listeners and events to trigger things.
My understanding is that they wanted to have Circles rolled out quickly and did not touch the code anymore since (in a breaking way at least). So, there was some way of public API classes defined that should not change. (This is something you could try to negotiate with the groupfolder team as well) In the meanwhile it is part of various documentation and tutorials and will not be changed in the near future/a quick way.
I think you have to distinguish between rules and best practices.
Rules are to prohibit bad behavior, like removing all data of users whose name start with a r. (You have a grunge with people with names starting with r due to bad experience with a foreign girlfriend. So you keep getting revenge. ) This is a broken rule and will get your app banned.
Best-practices should help you with the work. It shows how things are intended to be used and done. You are not obligated to respect these. Disrespecting them makes work harder for you and your fellows.
Sometimes, though, some use cases are not considered in the initial consideration and will not work within the best-practices. Then, you are asked to find a minimal invasive solution that sticks as close as possible in order to be maintainable. Keep it well documented and communicate it clearly.
I understand the idea… It could work !
Normally, the Nextcloud team would not come to me to say that isn’t a best practice and blacklist my app in the store ?
I think it’s a good idea to use this pattern type
I will study the question
I never use the Event and Listener system until now ^^’
So, I cannot tell whether it’s a good idea or not, I trust you
I don’t know for the Circles app. But, they have control over their software (server and theirs apps), so I think they can make exceptions with a few apps (?)
Yes, thank you for this distinction
With a funny example
But that’s precisely the point, Nextcloud didn’t define a rule (not a best practice) that you don’t touch \OCA namespaces ? Because, they consider those namespaces as a private namespace (like the \OC namespace) ?
Blacklisting for sure not. If there is e.g. a security issue, maybe but they will come to you before. They want to have support by the community and why shy away their supporters (devs)?
Yes, sort of private namespaces. I have nowhere read it formally written to be discouraged “only” in the forums and discussions (considered second-hand information).
Also the \OC namespace is not private but just not guaranteed to be stable. So your app would need much more version checks and more complicated code than necessary.
And as you say, the Circles app uses this approach officially. The same holds true with the calendars and the dav app (although there are changes on the horizon). So, I would not consider this a problem in the near future. If this would be banned, they need to provide better mechanisms until then.
Yeah, it’s a “pattern” I would like to learn ^^
Thanks for the wiki page
I followed your instructions and I succeed !
I tried this code and it works !
<?php
declare(strict_types=1);
namespace OCA\Workspace\Groupfolders;
use Exception;
use OCA\GroupFolders\Folder\FolderManager;
use OCA\GroupFolders\Service\DelegationService;
use OCP\AppFramework\Http\DataResponse;
use OCP\AppFramework\QueryException;
use OCP\Files\IRootFolder;
use OCP\Server;
use Psr\Container\ContainerInterface;
if (class_exists(FolderManager::class)) {
class Wrapper {
private FolderManager $fm;
private DelegationService $delegationServiceGroupFolders;
public function __construct(private IRootFolder $rootFolder)
{
try {
// $this->fm = $appContainer->get(FolderManager::class);
$this->fm = Server::get(FolderManager::class);
$this->delegationServiceGroupFolders = Server::get(DelegationService::class);
} catch (Exception $e) {
throw new \Exception($e->getMessage());
}
}
public function createFolder(string $mountpoint): int
{
return $this->fm->createFolder($mountpoint);
}
public function setFolderACL(int $folderId, bool $acl): void
{
$this->fm->setFolderACL($folderId, $acl);
}
public function addApplicableGroup(int $id, string $group): void
{
$this->fm->addApplicableGroup($id, $group);
}
public function setManageACL(int $folderId, string $type, string $id, bool $manageAcl): void
{
$this->fm->setManageACL($folderId, $type, $id, $manageAcl);
}
public function getFolder(int $id, int $rootStorageId): array
{
return $this->fm->getFolder($id, $rootStorageId);
}
private function formatFolder(array $folder): array {
$folder['group_details'] = $folder['groups'];
$folder['groups'] = array_map(function (array $group) {
return $group['permissions'];
}, $folder['groups']);
return $folder;
}
public function getAllFoldersWithSize(int $rootStorageId): array {
return $this->fm->getAllFoldersWithSize($rootStorageId);
}
private function getRootFolderStorageId(): ?int
{
return $this->rootFolder->getMountPoint()->getNumericStorageId();
}
}
} else {
class Wrapper {
private const MESSAGE = 'The groupfolder app was installed.';
public function __construct()
{
}
public function createFolder(string $mountpoint): int {
throw new \Exception(self::MESSAGE);
return 100;
}
public function setFolderACL(int $folderId, bool $acl): void {
throw new \Exception(self::MESSAGE);
}
public function addApplicableGroup(int $id, string $group): void
{
throw new \Exception(self::MESSAGE);
}
public function setManageACL(int $folderId, string $type, string $id, bool $manageAcl): void
{
throw new \Exception(self::MESSAGE);
}
public function getFolder(int $id, int $rootStorageId): array
{
throw new \Exception(self::MESSAGE);
return [];
}
}
}
I created a experiment branch on my remote and you can explore the code, if you want
Do you run into this exception? If not (I am pretty sure, you will not if you have the outer if in place), you can just use it in the constructor arguments as you did with the IRootFolder.
I would contact the author of the group folder app to clarify this. He/she should be aware of the problem and your solution so far.
I suggest to establish a way to communicate by events as these seem less brittle. Eventually, you can get him/her to define a NC-public API (in the server like e.g. Talk) but this is only possible in the long term. I would
go with the direct import in the short time scale to test things and make sure it works in general as expected.
Establish the event system in the mid time scale to be more robust and also define some sort of stable API
In the long run, you might to convince the author to define/fix a global interface (like the currently not existing OCP\Groupfolder\IGroupFolderManager with a public defined API).
Thank you very much for you help
Don’t hesitate if I can help you
We (Arawa) know Carl but, he left the Nextcloud company
So, we don’t know who maintains the Groupfolders app
I talked with my coworkers and we don’t want to use the direct import even if it’s in the short timescale. Because, we want our app is robust.
So, we would to try the Event / Listener system as a POC for our app.
But, if I understand this logic. That means we or Groupfolders’ developers have to write Events on the Groupfolders apps (GroupFolderCreateEvent, GroupfolderSetACLEvent, and so on. Or even GroupfolderEvent) and in the Workspace app, we have to write the Listeners on those events.
Is that it ?
You could just open an issue and see who might react. Alternatively, you can directly try to contact to most active users (icewind1991 and juliuishaertl).
You got it correct.
You could (for the sake of the POC) fork the groupfolder repo and hack in the relevant events. Then you could also present this as a starting point for the real groupfolder team.
Yes, I saw that yesterday and I will see with them if we need to create a Pull Request
Hmmm… In fact, I wonder if this is a good idea ?
Because, I need to create a groupfolder before create a workspace.
For example :
I call this API/REST with PostMan or Insomnia : POST - apps/workspace/spaces - with the “spacename” as POST variable.
My route redirect the request to the create method from WorkspaceController.
The WorkspaceController#create get the spacename variable
First, I create a groupfolder, then I create a workspace where I define the folder_id (the groupfolder identifier) as an argument to create a relationship between them.
So, I don’t think it’s a good solution, no ?
Because, with the Event and Listener system, I have to create Events on the Workspace side and Listeners on the Groupfolders side, no ?
You cannot get it atomic. So you will have to do one after the other.
And yes, the listener must be on the side of the groupfolder app. They could provide the events as well (like e.g. TriggerCreateGroupfolderEvent or similar). Then you could create and dispatch such an event.
You will however still run into the problem that your app will import OCA\Groupfolder classes (the event class!) in your code. So, you are only partially better than just calling the classes directly. This is a design flaw as the events are considered typed and thus you need a common code base. You could only prevent that if you used plain PHP types (strings/arrays/…), I guess.
However, it is better than just calling arbitrary classes as you restrict yourself to the set of methods/events that are actually stable and consistent.
This is in fact very similar to the approach discussed above where you import OCA\Groupfolder classes directly.
I have the impression that using OCA\Groupfolder is fairly general in Nextcloud ?
Because, if using the Events and Listeners system is close to good practice. We have to import a OCA\<appid> in any case.
As you said, it’s a design flaw (?)
I think this is where we have our limits in the Event / Listeners system.
Mybay I wrong but, I don’t know how to get a groupfolder from this step (it’s a pseudo code).
Yes, and I understand the Collective’s problem now.
When you said to take the short term solution. Do you think this solution can last 3 years ? 5 years ? Until we (Arawa and the Groupfolder’s developers or Nextcloud) found a solution ?
The problem arises even earlier but with your implementation on the groupware side: The need to use/import part of your code in order to understand the received event.
Your code is currently tailored down to your use case. This is fine to provide a POC but will not work 100% for the upstream app. They will not want to integrate a code snippet for every foreign app. But this can be addressed later on.
Most probably, I would say that they would need to deliver an event themselves with the group information (aka listener on your side) and your would get the folder as a payload. It would be sort of a ping-pong game with events.
But as you see, the types events make it necessary to have a common code base.
This can even last longer, I guess unless one app ceases to exist.
I’d say it is absolutely vital to define some classes that are not changed regularly. I am thinking of defining some classes in OCA\Groupfolder\API that provide very generic interfaces and that interfaces must not change much.
Each time, the API classes need to change their interface, you need to be very careful about compatibility. Maybe you should define in the very first moment a way to exchange the compatibility information (like a separate API version that you can trust to be semver-based). You will have to push out a new release once the API changes.
At least the group folder app seems rather stable (not much new features), so the API might be quite stable as well. So, there is a good chance that you will not see yourself in the situation to force out a hot fix release too often.
) This is a broken rule and will get your app banned.
