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

Fixing bugs in teampass API #4596

Open
wants to merge 5 commits into
base: master
Choose a base branch
from

Conversation

skt-jdrzik
Copy link

  • can not authentificate with json request
  • can not create item if it is in root folder - parent_id == 0
  • can not create duplicate item if it is enabled and it is not personal folder
  • bug if creating first item - throwing exception

- can not authentificate with json request
- can not create item if it is in root folder - parent_id == 0
- can not create duplicate item if it is enabled and it is not personal
  folder
- bug if creating first item - throwing exception
Comment on lines 62 to 68
$queryString = $request->getQueryString();
if ($request->getContentTypeFormat() != 'application/json') {
parse_str(html_entity_decode($queryString), $query);
return $this->sanitizeUrl($query);
}

return $request->toArray();
Copy link
Contributor

Choose a reason for hiding this comment

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

Can you add the missing space in indent?

Choose a reason for hiding this comment

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

where exaclty please? is it wrong formatting?

Copy link
Author

Choose a reason for hiding this comment

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

Changed in new patch

Copy link
Author

@skt-jdrzik skt-jdrzik Feb 25, 2025

Choose a reason for hiding this comment

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

I made typo in this code, I will fix it in next patch:
correct way is:
if ($request->getContentTypeFormat() != 'json')

@@ -342,8 +342,7 @@ private function checkForDuplicates(string $label, array $SETTINGS, array $itemI
);

if (DB::count() > 0 && (
(isset($SETTINGS['duplicate_item']) && (int) $SETTINGS['duplicate_item'] === 0)
|| (int) $itemInfos['personal_folder'] === 0)
Copy link
Contributor

Choose a reason for hiding this comment

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

This can't be better?
&& (int) $itemInfos['personal_folder'] === 0)

Copy link
Author

Choose a reason for hiding this comment

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

Changed to acceptable solution in new patch

@@ -73,7 +73,8 @@ public function createNewFolder(array $params): array
}

if (!$this->isParentFolderAllowed($parent_id, $user_accessible_folders, $user_is_admin)) {
return $this->errorResponse($this->lang->get('error_folder_not_allowed_for_this_user'));
if ($parent_id != 0 && $user_can_create_root_folder == false )
Copy link
Contributor

@corentin-soriano corentin-soriano Feb 20, 2025

Choose a reason for hiding this comment

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

This isn't checked in isParentFolderAllowed()?

Choose a reason for hiding this comment

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

What is vhevked in this context please?

Copy link
Author

Choose a reason for hiding this comment

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

I think if parent_id == 0 is not resolved in code, but I can be wrong

Copy link
Contributor

Choose a reason for hiding this comment

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

What is vhevked in this context please?

Typo -> "checked"

Copy link
Contributor

Choose a reason for hiding this comment

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

I think if parent_id == 0 is not resolved in code, but I can be wrong

This check could be added to isParentFolderAllowed() but not here: this is the responsibility of this method.

Copy link
Author

Choose a reason for hiding this comment

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

I have some work assignments, but if I have some free time I will fix it

@@ -350,7 +351,7 @@ private function updateUserFolderCache($tree, $title, $parent_id, $isPersonal, $
if (empty($cache_tree)) {
DB::insert(prefixTable('cache_tree'), [
'user_id' => $user_id,
'folders' => json_encode($newId),
'folders' => json_encode([$newId,]),
Copy link

@skt-mmisuth skt-mmisuth Feb 20, 2025

Choose a reason for hiding this comment

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

There seems to be a typo here as well, not sure , in [$newId,] is needed.

Copy link
Author

@skt-jdrzik skt-jdrzik Feb 21, 2025

Choose a reason for hiding this comment

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

Changed to array without comma in new patch

- indenting in getQueryString function
- duplicate item not allowed, comparison condition
- newId in folders.class.php changed to array without comma
@skt-jdrzik
Copy link
Author

I found some other bugs in teampass API code, which must be fixed:

  • AuthModel does not run buildUserFoldersList with all requests, only on first auth request
  • manageFolderPermissions in folders.class.php does not handle parent_id == 0

@skt-jdrzik
Copy link
Author

I found some other bugs in teampass API code, which must be fixed:

* AuthModel does not run buildUserFoldersList with all requests, only on first auth request

* manageFolderPermissions in folders.class.php does not handle parent_id == 0

First problem is hard to fix because, folders_list is encoded in JWT and if new folder is created and it is only added to JWT if user reauthentificate...

- BaseController typo in code 'application/json' instead 'json'
- user_can_create_root_folder add check to function isParentFolderAllowed
- manageFolderPermissions fix parent_id == 0 bug
@skt-jdrzik
Copy link
Author

I am attaching this script, to migrate from syspass to teampass and and there is also workaround for buildUserFoldersList which encoded folders_list in JWT and I reauthentificate always with creating new folder...

#!/usr/bin/python3

from syspass_api_client import api, account
from rich import print
import sys
import os
import httpx
import json

def teampass_api_key(username, password, apikey):
    url = f"{tp_url}/authorize"
    payload = { 'apikey': apikey,
                'login': username,
                'password': password}

    r = httpx.post(url, json=payload)
    response = json.loads(r.text)
#    print(response)
    return response["token"]

def create_item(token,label,folder_id,password,description,login,email,sys_url,tags,icon='fa-solid fa-start',anyone_can_modify=True):
    url = f"{tp_url}/item/create"
    payload = {
        'label' : label,
        'folder_id': folder_id,
        'password': password,
        'description': description,
        'login' : login,
        'email' : email,
        'url' : sys_url,
        'icon': icon,
        'tags': ','.join(tags),
        'anyone_can_modify': 1 if anyone_can_modify == True else 0,
        }

    r = httpx.post(url, json=payload, headers= { "Authorization" : f"Bearer {token}" })
    return json.loads(r.text)

def list_item_in_folder(token,folders=[1,]):
    url = f"{tp_url}/item/folders"
    payload = {
            'folder_list': ','.join(map(repr,folders)),
            'folders': json.dumps(folders)
        }

    r = httpx.get(url, params=payload, headers= { "Authorization": f"Bearer {token}" })
    print(r.text)
    return json.loads(r.text)

def add_folder(token, title, parent_id, complexity=0, duration='', create_auth_without=True, edit_auth_without=True, icon='', icon_selected='',access_rights='W'):
    url = f"{tp_url}/folder/create"
    payload= {
        'title': title,
        'parent_id': parent_id,
        'complexity': complexity,
        'duration': duration,
        'create_auth_without': '1' if create_auth_without == True else 0,
        'edit_auth_without': '1' if edit_auth_without == True else 0,
        'icon': icon,
        'icon_selected': icon_selected,
        'access_rights': access_rights
        }
    r = httpx.post(url, params=payload, headers= { "Authorization": f"Bearer {token}" })
    print(f"RESPONSE {r.text}")
    return json.loads(r.text)


category_ids = {}
folders_ids = {}

tp_url=os.getenv("TEAMPASS_URL")
tp_user=os.getenv("TEAMPASS_TOKEN_USER")
tp_api=os.getenv("TEAMPASS_TOKEN_APIKEY")
tp_pass=os.getenv("TEAMPASS_TOKEN_PASSWORD")

token = teampass_api_key(tp_user, tp_pass, tp_api)

o_api = api.JsonRpcApi()
o_account = account.Account(o_api)

response = add_folder(token,"SYSPASS-OLD",0)
if response["error"] != False:
  print("Error creating folder")
  sys.exit(1)
root_id = response["newId"]

token = teampass_api_key(tp_user, tp_pass, tp_api)

for account_data in o_account.search(count=500,tags_id=[]):
    passw = o_account.view_pass(account_id = account_data["account_id"])
    psw = passw["pass"]

    print(f"[green]Importing item: [/green] {account_data['account_name']}")
    print(f"[green]Customer: [/green] {account_data['customer_name']}")
    print(f"[green]Category: [/green] {account_data['category_name']}")
    print(f"[green]Group: [/green] {account_data['usergroup_name']}")
    print(f"[green]Tags: [/green] {account_data['tags']}")


    if (account_data["customer_name"] not in category_ids):
        response = add_folder(token,account_data["customer_name"],root_id)
        if response["error"] != False:
            print("Error creating folder")
            sys.exit(1)
        token = teampass_api_key(tp_user, tp_pass, tp_api)
        category_ids[account_data["customer_name"]] = response["newId"]
        category_id = response["newId"]
    else:
        category_id = category_ids[account_data["customer_name"]]

    if (category_id not in folders_ids):
        folders_ids[category_id] = {}

    if (account_data["category_name"] not in folders_ids[category_id]):
        response = add_folder(token,account_data["category_name"],category_id)
        if response["error"] != False:
            print("Error creating folder")
            sys.exit(1)

        token = teampass_api_key(tp_user, tp_pass, tp_api)
        folders_ids[category_id][account_data["category_name"]] = response["newId"]
        folder_id = response["newId"]
    else:
        folder_id = folders_ids[category_id][account_data["category_name"]]



    print(f"[green]Folder ID: [/green] {folder_id}")
    print("[red]#########################################[/red]")

    response = create_item(token,account_data["account_name"],folder_id,psw,account_data["account_notes"],account_data["account_login"],'SKT_IT_SYS@skytoll.sk',account_data["account_url"],account_data["tags"])

Copy link
Contributor

@corentin-soriano corentin-soriano left a comment

Choose a reason for hiding this comment

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

Generally, it is better to use === instead of == when possible.

@@ -60,7 +60,7 @@ public function getQueryStringParams()
{
$request = symfonyRequest::createFromGlobals();
$queryString = $request->getQueryString();
if ($request->getContentTypeFormat() != 'application/json') {
if ($request->getContentTypeFormat() != 'json') {
Copy link
Contributor

Choose a reason for hiding this comment

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

Indents must be 4 spaces.

@@ -375,7 +378,7 @@ private function updateUserFolderCache($tree, $title, $parent_id, $isPersonal, $
*/
private function manageFolderPermissions($parent_id, $newId, $user_roles, $access_rights, $user_is_admin)
{
if ($this->settings['subfolder_rights_as_parent'] ?? false) {
if ($patent_id != 0 && $this->settings['subfolder_rights_as_parent'] ?? false) {
Copy link
Contributor

Choose a reason for hiding this comment

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

patent_id -> parent_id

Copy link
Author

Choose a reason for hiding this comment

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

I have posted patch for typos but not tested yet, I will if I had some free time for it...

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.

3 participants