View Issue Details

IDProjectCategoryView StatusLast Update
0013538mantisbtbugtrackerpublic2014-09-23 18:05
ReporterDentxinho Assigned Todregad  
PrioritynormalSeverityminorReproducibilityrandom
Status closedResolutionfixed 
Product Version1.2.8 
Target Version1.2.9Fixed in Version1.2.9 
Summary0013538: API auth check on specific functions rework / not needed
Description

Functions access_has_bug_level, access_get_global_level and access_get_project_level from access_api.php calls

if( !auth_is_user_authenticated() ) {
return false;
}

before to call

if( null === $p_user_id ) {
$p_user_id = auth_get_current_user_id();
}

Doing so, we have troubles to develop plugins that e.g. checks if a specific user has a specific level (without being authenticated).
My suggestion is attached. I'm not familiar with git, so I can't make a pull request.

PS:
Strange that other accesshas and accessget functions doesn't have this type of validation. And if you see the code, you will see a @todo in these snippets. Does auth_is_user_authenticated is really needed?

These snippets were added on s:MantisBT:11587: and s:MantisBT:11513:

TagsNo tags attached.
Attached Files
api-auth-check-rework-not-needed.patch (1,932 bytes)   
diff --git a/access_api.php b/access_api.php
index 30ef237..db0b91c 100644
--- a/access_api.php
+++ b/access_api.php
@@ -221,10 +221,7 @@ function access_get_global_level( $p_user_id = null ) {
 		$p_user_id = auth_get_current_user_id();
 	}
 
-	# Deal with not logged in silently in this case
-	# @@@ we may be able to remove this and just error
-	#     and once we default to anon login, we can remove it for sure
-	if( !auth_is_user_authenticated() ) {
+	if ( empty($p_user_id) && !auth_is_user_authenticated() ) {
 		return false;
 	}
 
@@ -278,16 +275,14 @@ function access_ensure_global_level( $p_access_level, $p_user_id = null ) {
  * @access public
  */
 function access_get_project_level( $p_project_id = null, $p_user_id = null ) {
-	# Deal with not logged in silently in this case
-	/** @todo we may be able to remove this and just error and once we default to anon login, we can remove it for sure */
-	if( !auth_is_user_authenticated() ) {
-		return ANYBODY;
-	}
-
 	if( null === $p_user_id ) {
 		$p_user_id = auth_get_current_user_id();
 	}
 
+	if ( empty($p_user_id) && !auth_is_user_authenticated() ) {
+		return NOBODY;
+	}
+
 	if( null === $p_project_id ) {
 		$p_project_id = helper_get_current_project();
 	}
@@ -405,17 +400,14 @@ function access_has_any_project( $p_access_level, $p_user_id = null ) {
  * @access public
  */
 function access_has_bug_level( $p_access_level, $p_bug_id, $p_user_id = null ) {
-	# Deal with not logged in silently in this case
-	# @@@ we may be able to remove this and just error
-	#     and once we default to anon login, we can remove it for sure
-	if( !auth_is_user_authenticated() ) {
-		return false;
-	}
-
 	if( $p_user_id === null ) {
 		$p_user_id = auth_get_current_user_id();
 	}
 
+	if ( empty($p_user_id) && !auth_is_user_authenticated() ) {
+		return false;
+	}
+
 	$t_project_id = bug_get_field( $p_bug_id, 'project_id' );
 
 	# check limit_Reporter (Issue #4769)

Relationships

related to 0015721 closedgrangeway Functionality to consider porting to master-2.0.x 

Activities

Dentxinho

Dentxinho

2011-12-13 06:31

reporter   ~0030580

Please ignore the patch provided.

I've updated the code and sent a pull request on master-1.2.x

dregad

dregad

2011-12-13 17:49

developer   ~0030590

Thanks for the patch/pull request !

Dentxinho

Dentxinho

2012-01-23 06:46

reporter   ~0031003

What's happening with some issues? Are they cloning themselves?
I'm receiving emails with reports of duplicate issues :S

dregad

dregad

2012-01-23 07:55

developer   ~0031004

Please see http://news.gmane.org/gmane.comp.bug-tracking.mantis.devel for the explanation. Sorry for the inconvenience.

grangeway

grangeway

2013-04-05 17:57

reporter   ~0036363

Marking as 'acknowledged' not resolved/closed to track that change gets ported to master-2.0.x branch

Related Changesets

MantisBT: master f0248c84

2011-12-11 22:22

Dentxinho


Details Diff
Removed unneeded auth check in the access API.

Some functions on the Access API (access_get_global_level,
access_get_project_level and access_has_bug_level) require an
authenticated user in order to return correct values, FALSE otherwise.

However, these functions can be used by plugins while not
authenticated, so the code was changed to allow the execution to
proceed if existing parameter $p_user_id is provided.

Fixes 0013538

Signed-off-by: Damien Regad <damien.regad@merckgroup.com>

Original patch was modified to follow MantisBT coding guidelines and
improve the commit message
Affected Issues
0013538
mod - core/access_api.php Diff File

MantisBT: master-1.2.x 42eac59d

2011-12-11 22:22

Dentxinho


Details Diff
Removed unneeded auth check in the access API.

Some functions on the Access API (access_get_global_level,
access_get_project_level and access_has_bug_level) require an
authenticated user in order to return correct values, FALSE otherwise.

However, these functions can be used by plugins while not
authenticated, so the code was changed to allow the execution to
proceed if existing parameter $p_user_id is provided.

Fixes 0013538

Signed-off-by: Damien Regad <damien.regad@merckgroup.com>

Original patch was modified to follow MantisBT coding guidelines and
improve the commit message
Affected Issues
0013538
mod - core/access_api.php Diff File